Tweak xhr error handling #197
Conversation
|
Looks good, but I think needs @paddybyers's input for this one |
| err.code = 80000; | ||
| self.complete(err); | ||
| var errorHandler = function(errorEvent, message, code, statusCode) { | ||
| var errorMessage = message + ", errorEvent was " + Utils.inspect(errorEvent) + ", current statusText is " + self.xhr.statusText; |
There was a problem hiding this comment.
Single quotes please everywhere.
Also, have we checked that Utils.inspect() gives us something meaningful for the event? It's just JSON.stringify() so will it expose relevant information?
There was a problem hiding this comment.
Also, have we checked that Utils.inspect() gives us something meaningful for the event?
In my tests (just aborting or cutting the connection) it hasn't contained anything useful, no (the stringify was only {isTrusted: true}, and the rest of the event object didn't have anything better). Left it in there in the hope that maybe there's some more interesting error might happen in the wild where (a) the event object included some new fields, and (b) they'd be included in the stringify. Perhaps not very likely, but I figure leaving it there doesn't cost much. Want me to remove it?
There was a problem hiding this comment.
but I figure leaving it there doesn't cost much. Want me to remove it?
Looking at the event specs you're not going to get anything useful from the event object itself except the type of the event (ie error or abort). TBH I' remove the inspect() and just have 'type = ' + ev.type or something like that in the log message.
SimonWoolf
force-pushed
the
xhronerror
branch
from
79a3755
to
96e8ad2
Compare
|
Thanks |
Not sure whether bundling the errorEvent and the current statusText in there will actually be useful, but, shrug