Skip to content

refactor(js/core): restrict automatic shutdown signal handlers to development environment only#5157

Open
pavelgj wants to merge 4 commits intomainfrom
pj/no-exit
Open

refactor(js/core): restrict automatic shutdown signal handlers to development environment only#5157
pavelgj wants to merge 4 commits intomainfrom
pj/no-exit

Conversation

@pavelgj
Copy link
Copy Markdown
Member

@pavelgj pavelgj commented Apr 20, 2026

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the explicit process.exit(0) call from the shutdown signal handler in genkit.ts. The review feedback highlights that this change may cause the process to hang indefinitely if active handles remain in the event loop after the cleanup completes. Additionally, the reviewer suggests implementing concurrency protection for the asynchronous shutdown function to prevent race conditions if multiple signals are received.

Comment thread js/genkit/src/genkit.ts
@pavelgj pavelgj changed the title refactor: remove explicit process exit from Genkit shutdown sequence refactor(js/core): remove explicit process exit from Genkit shutdown sequence Apr 21, 2026
@pavelgj pavelgj changed the title refactor(js/core): remove explicit process exit from Genkit shutdown sequence refactor(js/core): restrict automatic shutdown signal handlers to development environment only Apr 21, 2026
@pavelgj
Copy link
Copy Markdown
Member Author

pavelgj commented Apr 21, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request restricts the registration of 'SIGTERM' and 'SIGINT' signal handlers to development environments to avoid interfering with the host application's shutdown behavior in production. The review feedback recommends adding error handling to the shutdown function to ensure that the process terminates even if the server cleanup fails.

Comment thread js/genkit/src/genkit.ts
@pavelgj pavelgj requested a review from MichaelDoyle April 24, 2026 14:57
Copy link
Copy Markdown
Contributor

@MichaelDoyle MichaelDoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

I think there are some handlers like this either in the google-cloud plugin or genkit instrumentation code to flush metrics as well, that we might want to take a second look at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants