oak
oak copied to clipboard
SSE (Server Sent Events) are getting "buffered" until initial function finishes execution
Just a small piece of feedback based on something I spend around 2 hours on
import {
Application,
Router,
ServerSentEvent,
} from 'https://deno.land/x/oak/mod.ts'
const app = new Application()
const router = new Router()
router.get('/sse', async (ctx) => {
const sse = ctx.sendEvents()
for (let i = 0; i < 10; i++) {
await new Promise((r) => setTimeout(r, 600))
sse.dispatchMessage({ hello: Math.random(), i })
}
})
app.use(router.routes())
await app.listen({ port: 8091 })
Will cause you to see all 10 messages arrive at the same time after 6 seconds rather than 1 by 1 every 600 ms ( https://great-deer-11.deno.dev/sse ). This is caused by the fact that ctx.sendEvents doesn't instantly start responding to the request, but it waits till the 'middleware handler function' finishes executing. This means that in practise right now you often need
const sse = ctx.sendEvents()
setTimeout(() => {
// actual code
}, 0);
I think it would either make sense to have ctx.sendEvents() instantly send the response if possible, or update the documentation to always include the setTimeout-hack.
I have the same issue, it's ugly and kinda breaks async/await with the setTimeout hack. But thanks for the hack.
This is actually working as designed. Ultimately SSE events shouldn't be dispatched within the main flow of the middleware, because the response is not sent to the client until all middleware has finished executing. Once you "break out" of the flow of the middleware, you can dispatch events as they occur.
So ultimately you want a pattern where you pass the target to some sort of event processor that then dispatches events on the target and does not block the execution of the rest of the middleware stack. The setTimeout() is one way to accomplish it, but basically anything that doesn't block the execution, sync or async, of the middleware would work.
I have added a note to the inline documentation about this.
This is kind of an unfortunate api to be honest, having to set a timeout which causes a new top level thread is not great. The error handling gets pretty complicated and can crash your server if you don't handle errors in that timeout...
I really feels weird that oak isn't expecting a promise based api to support this, I'm thinking that it would make sense for oak to support a function being passed which it then can properly handle errors and closing the stream properly, which is another issue I'm seeing.
I'm wondering if something like this would make sense? Can EventSource even support anything other than GET?
router.events('/sse', async (ctx, target) => {
for (let i = 0; i < 10; i++) {
if (target.closed) return;
await new Promise((r) => setTimeout(r, 600))
target.dispatchMessage({ hello: Math.random(), i })
}
})
Internally the router.events function could wrap the handler with setTimeout(fn, 0) and add top level error handling and hook into the Response and appropriately close the event target.
I understand the confusion, though the suggestion above isn't the right approach. As I stated above, dispatching events in the flow of the middleware isn't advisable, middleware is determining what the response is. There is also an assumption in the above that this would be a router thing, which a router is optional in oak and the solution needs to be addressed at the server level.
What can be done, which I will look at, is that we follow the same sort of path that we perform with upgrading to the websocket, which when this occurs a response is sent back to the client to complete the upgrade, the oak response is "locked" and the only way to communicate is via the returned target.
That all sounds reasonable, the above was just an illustration. The main request is just that there is an API better than wrapping the logic in a setTimeout, one that would have a pattern and reasonable error handling etc.
Ok, ctx.sendEvents() now works like the websocket .upgrade() where the response is immediately sent back to the client... because of the need to do this, the .sendEvents() returns a promise, therefore this is a breaking change. I will hopefully release oak v14 soon that incorporates this.