razshare / sveltekit-sse

Server Sent Events with SvelteKit
https://www.npmjs.com/package/sveltekit-sse
MIT License
301 stars 9 forks source link

Error: Controller is already closed #23

Closed OTheNonE closed 9 months ago

OTheNonE commented 9 months ago

The code works well as shown in README.md, but when i try to use setInterval() to emit the event as follows:

export function GET() {
    return event(async emit => {
        let i = 0;
        setInterval(() => emit("Count: "+i), 2000)
    }).toResponse()
}

I get the following error from server side:

node:internal/webstreams/readablestream:1067
      throw new ERR_INVALID_STATE.TypeError('Controller is already closed');
      ^

TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed
    at ReadableStreamDefaultController.enqueue (node:internal/webstreams/readablestream:1067:13)
    at emit (my-project/application/node_modules/sveltekit-sse/dist/events.js:58:16)
    at run (my-project/application/node_modules/sveltekit-sse/dist/event.js:15:14)
    at Timeout.eval [as _onTimeout] (my-project/application/src/routes/home/live-measurement/+server.ts:11:23)
    at listOnTimeout (node:internal/timers:573:17)
    at process.processTimers (node:internal/timers:514:7) {
  code: 'ERR_INVALID_STATE'
}

I cannot tell the exact error, but i notice that in the example shown in README.md, the run function is put into an endless loop and never finishes, while with my example, my arrow function actually finishes. Could this have something to do with the controller closing when the function finishes?

razshare commented 9 months ago

Hi @OTheNonE , whenever the event callback ends the stream closes and the response completes.

You need to lock your event resolver.

There are different ways to do that, in the README.md example I'm using an async infinite while loop

export function GET() {
  return event(async function run(emit){
    while (true) {   // as you can see this function never returns, which keeps the connection to the client open
      emit(`${Date.now()}`)
      await delay(1000)
    }
  }).toResponse()
}

In your use case you should be able to do it like this

export function GET() {
    return event(async emit => {
        let i = 0;
        await new Promise(resolve => {
            setInterval(() => {
                emit("Count: "+i)
                if(9 === i){
                    // this will complete the response after 10 iterations
                    resolve()
                }
                i++
            }, 2000)
        })
    }).toResponse()
}

Let me know if this is what you're looking for.


Edit

Obviously you don't have to complete your response after 10 iteration or at all, you can just keep it open (until the client closes it).

OTheNonE commented 9 months ago

Thank you for such a quick answer, amazing!

So this is the way to do it, keep the response alive by not letting it end? Feels kinda wierd, is this something that this library implements for making SSE work, or is this how SSE would work anyways with the barebone ReadableStream API?

I'll have a look at some of the examples in the "Issues" tab, i think the database example is sufficient to help me reach the goal.

Could a feature for the library be to handle the lock for us? So we only have to make a "setup" code for the event?

razshare commented 9 months ago

So this is the way to do it, keep the response alive by not letting it end? Feels kinda wierd, is this something that this library implements for making SSE work, or is this how SSE would work anyways with the barebone ReadableStream API?

Yes, because SSE works over http and SvelteKit implements platform standard apis for dealing with requests. That means it's using a ReadableStream to achieve SSE

https://github.com/tncrazvan/sveltekit-sse/blob/37681956db5228e5e4eb72bd0fefc87d011814a2/src/lib/events.js#L78-L91

Line 81 is waiting for your callback, the one you pass to event() and right under that it's closing the stream and the connection with controller.close().

Could a feature for the library be to handle the lock for us? So we only have to make a "setup" code for the event?

Yeah, that sounds doable.

I'll leave this issue open; I'll be back with an implementation and a new version.

razshare commented 9 months ago

Hi @OTheNonE , version 0.7.0 is out with a locking feature for both event() and events() and most importantly the main README.md has been updated showing a few examples of how to lock the event with both Promise and the new dedicated locking prop.

I'm closing this issue for now, feel free to open a new one if you're encountering any other issues.