Fix #175 by removing system.exit calls in the built-in handlers

[seratch]

This pull request resolves #175 and related reports. This commit 25165df in #45 introduced system.exit calls, which didn't exist before the fix. So, reverting them should make sense. An alternative approach would be to introduce a new flag env variable to turn this on/off but I don't think it's necessary.

[changeset-bot]

🦋 Changeset detected

Latest commit: c4351fe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@openai/agents-core	Patch
@openai/agents-openai	Patch
@openai/agents-realtime	Patch
@openai/agents	Patch
@openai/agents-extensions	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[nathanjcochran]

I think the gotcha here is that just registering the SIGINT/SIGTERM handlers changes Node's default signal handling behavior, causing it to no longer exit when it receives these signals. This is likely why the user's program was hanging in #44, and why the process.exit() calls were added in the first place in #45. See the Node.js docs, which say:

'SIGTERM' and 'SIGINT' have default handlers on non-Windows platforms that reset the terminal mode before exiting with code 128 + signal number. If one of these signals has a listener installed, its default behavior will be removed (Node.js will no longer exit).

The same is true for the unhandledRejection/uncaughtException events. Registering handlers for them changes the default behavior (which is to exit the process).

As a result, this PR will likely break apps that don't have their own handlers installed for those signals/events, because the presence of your handlers prevents them from exiting (which would be the normal/default Node.js behavior otherwise). On the flip side, including process.exit() calls in your handlers breaks apps that do have their own handlers installed for those signals/events, because in that case, the normal/default behavior would be for the app to not automatically exit (see #175).

At the end of the day, the difficulty is really just that you're registering signal handlers in a library package 😅. The only way to do it "correctly", as far as I can tell, is for your handlers to exit the process if no other handlers are installed for the relevant signal/event, but not to exit the process if there is at least one other handler registered for the signal/event. That would more-or-less preserve the default Node.js behavior, as explained in the docs I linked above.

[seratch]

Thanks for your comment; I will look into this later on.

[seratch]

Added check logic within those SDK built-in listeners and changed the behavior to exit the process only when there are no other listeners. This does not change the current behavior for simple code snippets + resolves the unintuitive process termination for apps with custom listeners.