ForestAdmin / forest-express

🧱 Dependency of Express Lianas for Forest Admin
GNU General Public License v3.0
72 stars 21 forks source link

Server events subscription inhibits graceful shutdown #1011

Closed ChrisLahaye closed 1 year ago

ChrisLahaye commented 1 year ago

Hello,

Applications with recent versions of forest-express, those that include the server events subscription, do not gracefully shut down.

This is caused by the commit https://github.com/ForestAdmin/forest-express/commit/b5db483116d574a7c2b168029d4d47103b77f887#diff-bfe9874d239014961b1ae4e89875a6155667db834a410aaaa2ebe3cf89820556R379.

This commit introduces an EventSource subscription which is never properly closed. Since this subscription opens resources that cannot accessed or closed upon shutdown, the process cannot gracefully exit.

How can we close this subscription?

ghusse commented 1 year ago

Hello @ChrisLahaye,

Thanks for reporting the issue. We'll take a look and keep you posted when it'll be fixed.

Thenkei commented 1 year ago

Hello @ChrisLahaye,

Thanks for the report. 🙏

Thanks for your answers. Morgan

ChrisLahaye commented 1 year ago

Hi @Thenkei!

Unfortunately I don't have logs to share because those were not emitted. About whether the SSE properly connects, I wouldn't know, I don't have any experience with EventSource. I do know that my process only terminates gracefully when commenting out the line await inject().forestAdminClient.subscribeToServerEvents(), that the EventSource has a close function which isn't called or exposed, and that EventSource probably creates handles, similarly as websockets, timers, and timeouts, that need to be properly closed for the process to gracefully exit.

Thenkei commented 1 year ago

We deactivated the EventSource for now. It didn't provide any significantly different experience on the agent forest-express-sequelize and forest-express-mongoose.

We just released some new versions of our agents.

Let me know if it's ok on your side. Again, thanks for the report 🙏

thongxuan commented 1 year ago

We pumped to forest-express-mongoose v9.3.2 and the issue is fixed. Thank you 😇