snoyberg
Default value of onRequestBodyException causes exceptions to be ignored when streaming body
The default onRequestBodyException is:
onRequestBodyException = \se ->
case E.fromException se of
Just (_ :: IOException) -> return ()
Nothing -> throwIO se
The problem is that when using the streaming or chunked streaming body, if an IOException is thrown, it is ignored. The problem is that it means that if I'm e.g. streaming the body from a file (e.g. using amazonka-s3 and default conduit) and an IOException occurs (e.g. Permission denied), the whole request just hangs.
I'm probably going to solve this by recasting the exception to something else (and writing my own file-reading conduit), as there seems to be no way to override this default in amazonka. However it seems to me a rather weird default, does it have any reason to be there?
It's a weird case, but the description is in the Git commit: ae82e09c4658babf0aca7edd98669f0b941beeae. For the case you describe, I would recommend opening up the file handle before starting the Request, if possible.
Perhaps we could be more specific in which IOExceptions we ignore here.
My plan is to ultimately stream directly from an FTP connection, so I don't think that would help. However, it seems to me it would be better to solve it the 'reverse' way I did it in my code - what about catching the IO exception in the writeBuilder, rethrowing it as some custom exception, catching that, if it is an early close, then ignore, otherwise rethrow it as a normal exception again?
I hear that, and in a from-scratch implementation that would probably be a better approach. However, in this case, I'm concerned about such a subtle change to behavior that's been around for a very long time in the library.
I wonder if some code depends on the errror from the RequestBodyStream to be ignored or handled in the onRequestBodyException. The 'Workflow' xkcd strip comes to my mind, but still....
Wouldn't it at least warrant some big warning in the documentation of the RequestBodyStream? It took a me a few hours to trace this down and I'd like to save the time of other people ;)
Definitely docs. I'm not even saying no to the change, I'm just saying I'm concerned about it. At the very least a public announcement of the change would be worthwhile. I'm a bit on the fence still though.
How's this: would you be up for putting out a message on the yesodweb.com blog proposing the change (maybe even just a link to a proposed PR) and we'll see if anyone has complaints? That seems like a good way to get publicity on this before making a change.