Exceptions from AsyncGenerator responses cause the connection to hang #659
Comments
|
Update: One of the great things about [Async]Generators is how composable they are. Unfortunately, that means my proposed workaround may work in fewer cases than I thought, and doesn't work in mine. While the core of my logic does look like my //pseudocode
async function * pageBoilerplate(pageContents) {
yield `<!doctype html>\n`
yield * otherPageBoilerplate()
for await (const item of pageContents()) { yield item }
// etc.
}So a peekable is only going to get that I may need to rearchitect things a bit to avoid this bug. |
|
🤦 Nevermind. The "connection being left open indefinitely" issue was not due to oak's handling of A simple test like this shows that Oak does close the connection when an AsyncIterable throws: function example({response}: oak.Context) {
const body = async function * renderBody(): AsyncGenerator<string> {
yield `First\n`
yield `Second\n`
console.log("Got here")
throw new Error(`Error while rendering body`)
}
response.type = "text/plain"
response.body = body()
}It is still true that this exception doesn't get logged by the documented error logging pattern, but we can open that as a smaller issue w/o all the unrelated context in this ticket. Sorry for the noise! |
The Bug
When
ctx.response.bodyis set to an AsyncGenerator, if that generator throws,then the HTTP connection is left open indefinitely.Additionally, the example pattern for error logging does not log the Error.
Oak should:
at a minimum:
preferably:
yield, set an HTTP 500 so the end user has some indication that there was a server-side error.Context
I'm using this pattern for a server I'm writing:
If
initialValidation()throws, then the error logging will print the error.However, if it's the AsyncGenerator that throws, the error is not logged, and the connection remains open.
I have an idea as to why this is the case -- the middleware call stack is fully exited before the AsyncGenerator starts being polled by Oak to send the response. i.e.: the error is thrown after
next()resolves, so middleware can't catch it.Possible Workaround
I haven't confirmed it yet, but I expect most of these types of errors can probably be promoted to happen before the next() resolves by setting
ctx.response.bodyto be a "peekable" AsyncIterable, and by "peeking" at the first yielded value before returning from middleware.That's because I bet most of these async iterables will be implemented something like:
And I bet most of the errors are going to be related to I/O (network errors, SQL query errors, etc.), so will happen before the first yield.
By "peeking" at the first value before returning from middleware, we can cause that exception to happen earlier, which will then be handled by logging/error middleware.
Of course, this doesn't handle all cases, so we'd still want Oak to catch those and at least close the connection.
Shameless plug, I'm going to use my own peekable implementation to try to work around this for myself.
The text was updated successfully, but these errors were encountered: