capture async error in writebody(http, request, body) #285
Conversation
Codecov Report
@@ Coverage Diff @@
## master #285 +/- ##
=========================================
Coverage ? 90.37%
=========================================
Files ? 29
Lines ? 1185
Branches ? 0
=========================================
Hits ? 1071
Misses ? 114
Partials ? 0
Continue to review full report at Codecov.
|
| catch e | ||
| write_error = e | ||
| @warn("Error in @async writebody task", | ||
| exception=(write_error, catch_backtrace())) |
There was a problem hiding this comment.
I think this should be stacktrace(catch_backtrace())
There was a problem hiding this comment.
I did it this way because the @warn macro seems to do special processing of Tuple{Exception,BackTrace}. i.e. it prints out a nice stack-trace.
src/StreamRequest.jl
Outdated
| if e isa CompositeException | ||
| e = first(e.exceptions).ex | ||
| if write_error != nothing | ||
| throw(write_error) |
There was a problem hiding this comment.
Is this changing behavior from before? Didn't we pass on to the next if block before and check is it's a isioerror. Why change that now?
There was a problem hiding this comment.
hmm, looking at this again, I think there is more to it...
Assume that writebody is sending some huge message body, and the server sends an error with Connection: "close (https://tools.ietf.org/html/rfc7230#section-6.5).
readbody will return, and isaborted() will be true on line 72.
So, close gets called and the writebody task stop with an IO error, which is stored in write_error.
However, the catch block at 79 will never be reached, because there is no exception to catch in the main task, so write_error will not be used at all. This makes sense in this case because the main task has decided to kill the writebody task and doesn't care about any subsequent exceptions that it might throw.
I think we can reduce the catch block to just:
catch e
if write_error != nothing
throw(write_error)
else
rethrow(e)
end
endThis makes the intent of the code a bit easier to follow, i.e: If there is an error while reading the response, check if the root-cause was an error sending the request, otherwise just rethrow the reading error.
|
Rebased. |
See #245 (comment)