Added custom SignalR error handling capability - Fixed #515 #585

Merged
merged 7 commits into from Aug 14, 2012

Projects

None yet

2 participants

Contributor

Fixed #515

Created an ErrorExtensions class to add a "GetError" capability to
Exceptions. GetError returns an error of type SignalRError which
unwraps and pulls unique attributes out of a thrown exception to expose
essential pieces to errors.

NTaylorMullen added some commits Aug 7, 2012
@NTaylorMullen NTaylorMullen Unwrapped thrown exceptions
Developers were getting wrapped exceptions which were not very
distinguishable and required a lot of digging to pull out the base
exception which is the most relevant piece.
cfdd04c
@NTaylorMullen NTaylorMullen Added custom SignalR error handling capability
Fixed #515

Created an ErrorExtensions class to add a "GetError" capability to
Exceptions.  GetError returns an error of type SignalRError which
unwraps and pulls unique attributes out of a thrown exception to expose
essential pieces to errors.
b590bb0
@NTaylorMullen NTaylorMullen Disposed of cloned stream after usage ad6a1f8
@davidfowl davidfowl and 1 other commented on an outdated diff Aug 9, 2012
SignalR.Client/Infrastructure/ErrorExtensions.cs
+ return error;
+ }
+
+ private static Stream Clone(Stream source)
+ {
+ Stream cloned = new MemoryStream();
+ byte[] buffer = new byte[2048];// Copy up to 2048 bytes at a time
+ int copiedBytes;// Maintains how many bytes were read
+
+ while ((copiedBytes = source.Read(buffer,0,buffer.Length)) > 0)// Read bytes and copy them into a buffer making sure not to trigger the dispose
+ {
+ cloned.Write(buffer, 0, copiedBytes);// Write the copied bytes from the buffer into the cloned stream
+ }
+
+ // Move the stream pointers back to the original start locations
+ source.Seek(0, 0);
davidfowl
davidfowl Aug 9, 2012 Owner

Check if CanSeek is true on the source.

NTaylorMullen
NTaylorMullen Aug 9, 2012 Contributor

CanSeek is set to true. If we use ReadToEnd the CanSeek turns to false, however if we read the stream piece by piece with Read it retains the CanSeek value.

davidfowl
davidfowl Aug 11, 2012 Owner

This still bugs me but maybe it works out just fine.

@davidfowl davidfowl commented on an outdated diff Aug 13, 2012
SignalR.Client/Infrastructure/ErrorExtensions.cs
+ Stream originStream = response.GetResponseStream();
+
+ if (originStream.CanRead)
+ {
+ // We need to copy the stream over and not consume it all on "ReadToEnd". If we consumed the entire stream GetError
+ // would only be able to be called once per Exception, otherwise you get inconsistent ResponseBody results.
+ Stream stream = Clone(originStream);
+
+ // Consume our copied stream
+ using (var sr = new StreamReader(stream))
+ {
+ error.ResponseBody = sr.ReadToEnd();
+ }
+
+ //Free up the resources for the cloned stream
+ stream.Dispose();
davidfowl
davidfowl Aug 13, 2012 Owner

remove this

@davidfowl davidfowl commented on an outdated diff Aug 13, 2012
SignalR.Client/Infrastructure/ErrorExtensions.cs
+ {
+ error.ResponseBody = sr.ReadToEnd();
+ }
+
+ //Free up the resources for the cloned stream
+ stream.Dispose();
+ }
+ }
+ }
+
+ return error;
+ }
+
+ private static Stream Clone(Stream source)
+ {
+ Stream cloned = new MemoryStream();
@davidfowl davidfowl commented on an outdated diff Aug 13, 2012
SignalR.Client/Infrastructure/ErrorExtensions.cs
+ //Free up the resources for the cloned stream
+ stream.Dispose();
+ }
+ }
+ }
+
+ return error;
+ }
+
+ private static Stream Clone(Stream source)
+ {
+ Stream cloned = new MemoryStream();
+ byte[] buffer = new byte[2048];// Copy up to 2048 bytes at a time
+ int copiedBytes;// Maintains how many bytes were read
+
+ while ((copiedBytes = source.Read(buffer,0,buffer.Length)) > 0)// Read bytes and copy them into a buffer making sure not to trigger the dispose
davidfowl
davidfowl Aug 13, 2012 Owner

formatting.

NTaylorMullen added some commits Aug 13, 2012
@NTaylorMullen NTaylorMullen Addressed PR comments
Made the SignalRError object disposable to assure no memory is leaked
via the HttpWebResponse associated with WebExceptions
7a83108
@NTaylorMullen NTaylorMullen Updated unit test to use new error handling 35f252d
@NTaylorMullen NTaylorMullen Pointed unit test url to new URL
The prior URL was pointing to localhost and if an IIS server was not
running the machine would actively refuse the connection, resulting in a
failure of the test (not a 404).
a2bdf66
@NTaylorMullen NTaylorMullen Modified inline comments c414a16
@davidfowl davidfowl commented on the diff Aug 14, 2012
SignalR.Client/Infrastructure/SignalRError.cs
+ private HttpWebResponse _response;
+
+ /// <summary>
+ /// Create custom SignalR based error.
+ /// </summary>
+ /// <param name="exception">The exception to unwrap</param>
+ public SignalRError(Exception exception)
+ {
+ Exception = exception;
+ }
+
+ /// <summary>
+ /// Capture the response so we can dispose of it later
+ /// </summary>
+ /// <param name="response">The HttpWebResponse associated with the Exception, assuming its a WebException</param>
+ public void SetResponse(HttpWebResponse response)
davidfowl
davidfowl Aug 14, 2012 Owner

Make this internal.

@davidfowl davidfowl merged commit aea3e61 into 0.5.3 Aug 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment