This resolves #883. The previous behavior when the WebSocket was closed with a 4006 (invalid state) was to kill the reconnect task and log an exception. While this behavior is 'correct', it does not make it obvious to the user (or developer, in my case) as to why their client has just quit and not bothered to attempt a reconnect. The current best way to determine if the client needs to be reset is to check that CloseCode property on the WebSocketException is set to 4006. However, since Discord does not have close code 4006 documented, it feels like only half of a solution. This change allows users to opt-out of fatal invalid-states in the client's configuration - though it is discouraged. If they have not opted out of them, a 4006 will trigger a FatalException, which can then be caught with a more simple pattern match in the Log event. This change also expands what we consider to be a fatal close code, and will raise a FatalException for a 1000 (Discord should never close a connection for us), 4004 (invalid token), 4010/4011 (shard related).pull/927/head
@@ -0,0 +1,12 @@ | |||||
using System; | |||||
namespace Discord.Net | |||||
{ | |||||
public class FatalException : Exception | |||||
{ | |||||
public FatalException(string reason, Exception inner) | |||||
: base(reason, inner) | |||||
{ | |||||
} | |||||
} | |||||
} |
@@ -1,4 +1,5 @@ | |||||
using System; | |||||
using System; | |||||
namespace Discord.Net | namespace Discord.Net | ||||
{ | { | ||||
public class WebSocketClosedException : Exception | public class WebSocketClosedException : Exception | ||||
@@ -6,7 +7,7 @@ namespace Discord.Net | |||||
public int CloseCode { get; } | public int CloseCode { get; } | ||||
public string Reason { get; } | public string Reason { get; } | ||||
public WebSocketClosedException(int closeCode, string reason = null) | |||||
public WebSocketClosedException(int closeCode, string reason = null, bool fatal = false) | |||||
: base($"The server sent close {closeCode}{(reason != null ? $": \"{reason}\"" : "")}") | : base($"The server sent close {closeCode}{(reason != null ? $": \"{reason}\"" : "")}") | ||||
{ | { | ||||
CloseCode = closeCode; | CloseCode = closeCode; | ||||
@@ -1,4 +1,4 @@ | |||||
using Discord.API.Voice; | |||||
using Discord.API.Voice; | |||||
using Discord.Audio.Streams; | using Discord.Audio.Streams; | ||||
using Discord.Logging; | using Discord.Logging; | ||||
using Discord.Net.Converters; | using Discord.Net.Converters; | ||||
@@ -71,7 +71,7 @@ namespace Discord.Audio | |||||
ApiClient.ReceivedPacket += ProcessPacketAsync; | ApiClient.ReceivedPacket += ProcessPacketAsync; | ||||
_stateLock = new SemaphoreSlim(1, 1); | _stateLock = new SemaphoreSlim(1, 1); | ||||
_connection = new ConnectionManager(_stateLock, _audioLogger, 30000, | |||||
_connection = new ConnectionManager(_stateLock, _audioLogger, 30000, false, | |||||
OnConnectingAsync, OnDisconnectingAsync, x => ApiClient.Disconnected += x); | OnConnectingAsync, OnDisconnectingAsync, x => ApiClient.Disconnected += x); | ||||
_connection.Connected += () => _connectedEvent.InvokeAsync(); | _connection.Connected += () => _connectedEvent.InvokeAsync(); | ||||
_connection.Disconnected += (ex, recon) => _disconnectedEvent.InvokeAsync(ex); | _connection.Disconnected += (ex, recon) => _disconnectedEvent.InvokeAsync(ex); | ||||
@@ -1,13 +1,17 @@ | |||||
using Discord.Logging; | using Discord.Logging; | ||||
using Discord.Net; | |||||
using System; | using System; | ||||
using System.Threading; | using System.Threading; | ||||
using System.Threading.Tasks; | using System.Threading.Tasks; | ||||
using Discord.Net; | |||||
using System.Linq; | |||||
namespace Discord | namespace Discord | ||||
{ | { | ||||
internal class ConnectionManager | internal class ConnectionManager | ||||
{ | { | ||||
// close codes that cannot be recovered from | |||||
private static readonly int[] _fatalErrorCodes = { 4004, 4010, 4011 }; | |||||
public event Func<Task> Connected { add { _connectedEvent.Add(value); } remove { _connectedEvent.Remove(value); } } | public event Func<Task> Connected { add { _connectedEvent.Add(value); } remove { _connectedEvent.Remove(value); } } | ||||
private readonly AsyncEvent<Func<Task>> _connectedEvent = new AsyncEvent<Func<Task>>(); | private readonly AsyncEvent<Func<Task>> _connectedEvent = new AsyncEvent<Func<Task>>(); | ||||
public event Func<Exception, bool, Task> Disconnected { add { _disconnectedEvent.Add(value); } remove { _disconnectedEvent.Remove(value); } } | public event Func<Exception, bool, Task> Disconnected { add { _disconnectedEvent.Add(value); } remove { _disconnectedEvent.Remove(value); } } | ||||
@@ -26,7 +30,7 @@ namespace Discord | |||||
public ConnectionState State { get; private set; } | public ConnectionState State { get; private set; } | ||||
public CancellationToken CancelToken { get; private set; } | public CancellationToken CancelToken { get; private set; } | ||||
internal ConnectionManager(SemaphoreSlim stateLock, Logger logger, int connectionTimeout, | |||||
internal ConnectionManager(SemaphoreSlim stateLock, Logger logger, int connectionTimeout, bool invalidStateFatal, | |||||
Func<Task> onConnecting, Func<Exception, Task> onDisconnecting, Action<Func<Exception, Task>> clientDisconnectHandler) | Func<Task> onConnecting, Func<Exception, Task> onDisconnecting, Action<Func<Exception, Task>> clientDisconnectHandler) | ||||
{ | { | ||||
_stateLock = stateLock; | _stateLock = stateLock; | ||||
@@ -40,8 +44,8 @@ namespace Discord | |||||
if (ex != null) | if (ex != null) | ||||
{ | { | ||||
var ex2 = ex as WebSocketClosedException; | var ex2 = ex as WebSocketClosedException; | ||||
if (ex2?.CloseCode == 4006) | |||||
CriticalError(new Exception("WebSocket session expired", ex)); | |||||
if ((invalidStateFatal && ex2?.CloseCode == 4006) || _fatalErrorCodes.Contains(ex2?.CloseCode ?? 0)) | |||||
FatalError(new FatalException("WebSocket connection was closed with an unrecoverable error", ex)); | |||||
else | else | ||||
Error(new Exception("WebSocket connection was closed", ex)); | Error(new Exception("WebSocket connection was closed", ex)); | ||||
} | } | ||||
@@ -186,7 +190,7 @@ namespace Discord | |||||
_connectionPromise.TrySetException(ex); | _connectionPromise.TrySetException(ex); | ||||
_connectionCancelToken?.Cancel(); | _connectionCancelToken?.Cancel(); | ||||
} | } | ||||
public void CriticalError(Exception ex) | |||||
public void FatalError(Exception ex) | |||||
{ | { | ||||
_reconnectCancelToken?.Cancel(); | _reconnectCancelToken?.Cancel(); | ||||
Error(ex); | Error(ex); | ||||
@@ -207,4 +211,4 @@ namespace Discord | |||||
} | } | ||||
} | } | ||||
} | } | ||||
} | |||||
} |
@@ -1,4 +1,4 @@ | |||||
#pragma warning disable CS1591 | |||||
#pragma warning disable CS1591 | |||||
using Discord.API.Gateway; | using Discord.API.Gateway; | ||||
using Discord.API.Rest; | using Discord.API.Rest; | ||||
using Discord.Net.Queue; | using Discord.Net.Queue; | ||||
@@ -90,7 +90,7 @@ namespace Discord.WebSocket | |||||
_stateLock = new SemaphoreSlim(1, 1); | _stateLock = new SemaphoreSlim(1, 1); | ||||
_gatewayLogger = LogManager.CreateLogger(ShardId == 0 && TotalShards == 1 ? "Gateway" : $"Shard #{ShardId}"); | _gatewayLogger = LogManager.CreateLogger(ShardId == 0 && TotalShards == 1 ? "Gateway" : $"Shard #{ShardId}"); | ||||
_connection = new ConnectionManager(_stateLock, _gatewayLogger, config.ConnectionTimeout, | |||||
_connection = new ConnectionManager(_stateLock, _gatewayLogger, config.ConnectionTimeout, config.InvalidStateFatal, | |||||
OnConnectingAsync, OnDisconnectingAsync, x => ApiClient.Disconnected += x); | OnConnectingAsync, OnDisconnectingAsync, x => ApiClient.Disconnected += x); | ||||
_connection.Connected += () => TimedInvokeAsync(_connectedEvent, nameof(Connected)); | _connection.Connected += () => TimedInvokeAsync(_connectedEvent, nameof(Connected)); | ||||
_connection.Disconnected += (ex, recon) => TimedInvokeAsync(_disconnectedEvent, nameof(Disconnected), ex); | _connection.Disconnected += (ex, recon) => TimedInvokeAsync(_disconnectedEvent, nameof(Disconnected), ex); | ||||
@@ -466,7 +466,7 @@ namespace Discord.WebSocket | |||||
} | } | ||||
catch (Exception ex) | catch (Exception ex) | ||||
{ | { | ||||
_connection.CriticalError(new Exception("Processing READY failed", ex)); | |||||
_connection.FatalError(new Exception("Processing READY failed", ex)); | |||||
return; | return; | ||||
} | } | ||||
@@ -1,4 +1,4 @@ | |||||
using Discord.Net.Udp; | |||||
using Discord.Net.Udp; | |||||
using Discord.Net.WebSockets; | using Discord.Net.WebSockets; | ||||
using Discord.Rest; | using Discord.Rest; | ||||
@@ -25,6 +25,10 @@ namespace Discord.WebSocket | |||||
/// Gets or sets the max number of users a guild may have for offline users to be included in the READY packet. Max is 250. | /// Gets or sets the max number of users a guild may have for offline users to be included in the READY packet. Max is 250. | ||||
/// </summary> | /// </summary> | ||||
public int LargeThreshold { get; set; } = 250; | public int LargeThreshold { get; set; } = 250; | ||||
/// <summary> | |||||
/// Gets or sets whether a State Invalidation from Discord should be fatal. Setting this to false could lead to unexpected behavior when Discord is unstable. | |||||
/// </summary> | |||||
public bool InvalidStateFatal { get; set; } = true; | |||||
/// <summary> Gets or sets the provider used to generate new websocket connections. </summary> | /// <summary> Gets or sets the provider used to generate new websocket connections. </summary> | ||||
public WebSocketProvider WebSocketProvider { get; set; } | public WebSocketProvider WebSocketProvider { get; set; } | ||||