Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Swallow some exceptions on the server so they don't get logged #2450

Closed
davidfowl opened this Issue Aug 21, 2013 · 4 comments

Comments

Projects
None yet
5 participants
Owner

davidfowl commented Aug 21, 2013

We've always had several reports of users complaining that their logs are full of errors they can't filter because of exceptions that occur when using SignalR. These aren't actually a problem, but the fact that they show up in logs makes people worry about them and makes them file bugs. The best thing to do would be to swallow these errors trace them and return a non faulted task (if possible).

Examples of bug reports
#1942
#2447
#2393
#1998
#2544

Contributor

jcondex commented Sep 4, 2013

We should also swallow this one (C# client):

StressTestLogger Critical: 116 : [17:45:12] System.Net.WebSockets.WebSocketException (0x80004005): An internal WebSocket error occurred. Please see the innerException, if present, for more details.  ---> System.Net.Sockets.SocketException (0x80004005): An existing connection was forcibly closed by the remote host
   at System.Net.WebSockets.WebSocketConnectionStream.WebSocketConnection.ReadAsyncCore(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken, Boolean ignoreReadError)
   at System.Net.WebSockets.WebSocketConnectionStream.WebSocketConnection.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
   at System.Net.DelegatedStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
   at System.Net.WebSockets.WebSocketConnectionStream.<ReadAsync>d__c.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Net.WebSockets.WebSocketBase.WebSocketOperation.<Process>d__47.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Net.WebSockets.WebSocketBase.<ReceiveAsyncCore>d__1.MoveNext()
   at System.Net.WebSockets.WebSocketBase.ThrowIfConvertibleException(String methodName, Exception exception, CancellationToken cancellationToken, Boolean aborted)
   at System.Net.WebSockets.WebSocketBase.<ReceiveAsyncCore>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at Microsoft.AspNet.SignalR.WebSockets.WebSocketMessageReader.<ReadMessageAsync>d__0.MoveNext() in c:\Users\jconde\Documents\GitHub\SignalR\src\Microsoft.AspNet.SignalR.Core\Owin\WebSockets\WebSocketMessageReader.cs:line 33
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Microsoft.AspNet.SignalR.WebSockets.WebSocketHandler.<ProcessWebSocketRequestAsync>d__e.MoveNext() in c:\Users\jconde\Documents\GitHub\SignalR\src\Microsoft.AspNet.SignalR.Core\Owin\WebSockets\WebSocketHandler.cs:line 163

@ghost ghost assigned davidfowl Oct 15, 2013

@ghost ghost assigned halter73 Oct 31, 2013

halter73 added a commit that referenced this issue Oct 31, 2013

Respond to bad requests with a 400 status code
- Previously bad requests caused PersistentConnection.ProcessRequest to throw
  resulting in a 500
- Respond with a 403 status code when the client changes identity
- No longer throwing on bad requests reduces extraneous error logging

#2450

halter73 added a commit that referenced this issue Oct 31, 2013

Traces logged by TaskAsynceHelper.Catch are now warnings
- This should make it easier to filter out those traces

#2450

halter73 added a commit that referenced this issue Nov 1, 2013

Respond to bad requests with a 400 status code
- Previously bad requests caused PersistentConnection.ProcessRequest to throw
  resulting in a 500
- Respond with a 403 status code when the client changes identity
- No longer throwing on bad requests reduces extraneous error logging

#2450

halter73 added a commit that referenced this issue Nov 1, 2013

Traces logged by TaskAsynceHelper.Catch are now warnings
- This should make it easier to filter out those traces

#2450

halter73 added a commit that referenced this issue Nov 1, 2013

Respond to bad requests with a 400 status code
- Previously bad requests caused PersistentConnection.ProcessRequest to throw
  resulting in a 500
- Respond with a 403 status code when the client changes identity
- No longer throwing on bad requests reduces extraneous error logging

#2450

halter73 added a commit that referenced this issue Nov 1, 2013

Traces logged by TaskAsynceHelper.Catch are now warnings
- This should make it easier to filter out those traces

#2450

halter73 added a commit that referenced this issue Nov 8, 2013

Respond to bad requests with a 400 status code
- Previously bad requests caused PersistentConnection.ProcessRequest to throw
  resulting in a 500
- Respond with a 403 status code when the client changes identity
- No longer throwing on bad requests reduces extraneous error logging

#2450

halter73 added a commit that referenced this issue Nov 8, 2013

Traces logged by TaskAsynceHelper.Catch are now warnings
- This should make it easier to filter out those traces

#2450
Contributor

gustavo-armenta commented Dec 17, 2013

using Fiddler, tested invalid websocket requests return a 400 and there is no error on log file

Owner

davidfowl commented Jan 16, 2014

@gustavo-armenta We tested all the other errors too right?

Contributor

gustavo-armenta commented Jan 21, 2014

all other errors respond with 400 or 403 and an error message.

@davidfowl
#1942 and #2447 throw during ServerResponse.Flush() and the error still appears in the log. I think we should keep the behavior as in this case it is not possible to send a different response status code and error message, the response is already in a corrupted state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment