You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Calling eventStream.onClosed() in the current implementation registers a callback on both event.node.res.on('close') and writer.closed. This means that the callback will always fire twice, which could lead to some unexpected side effects, esp if users are interacting with things like database connections.
Additional context
We are already listening for event.node.res.on('close') by default (when autoclose is set to true). So, I propose we remove event.node.res.on('close') from the onClosed method because that case is already being handled in the default configuration.
I think if someone has chosen to set autoclose to false, they have already decided to take on additional work when it comes to cleanup. They can still manually register a callback on event.node.res.on('close') if they want.
Logs
No response
The text was updated successfully, but these errors were encountered:
Environment
Node:
v20.11.1
H3:
v1.11.1
Reproduction
https://github.com/joshmossas/h3-sse-on-closed-bug
Describe the bug
Calling
eventStream.onClosed()
in the current implementation registers a callback on bothevent.node.res.on('close')
andwriter.closed
. This means that the callback will always fire twice, which could lead to some unexpected side effects, esp if users are interacting with things like database connections.Additional context
We are already listening for
event.node.res.on('close')
by default (whenautoclose
is set to true). So, I propose we removeevent.node.res.on('close')
from theonClosed
method because that case is already being handled in the default configuration.I think if someone has chosen to set
autoclose
to false, they have already decided to take on additional work when it comes to cleanup. They can still manually register a callback onevent.node.res.on('close')
if they want.Logs
No response
The text was updated successfully, but these errors were encountered: