Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Squashing client and worker shutdown errors #40

Merged
merged 2 commits into from
Aug 13, 2023

Conversation

clintonb
Copy link
Contributor

@clintonb clintonb commented Jun 9, 2023

If the client connection or worker is not cleanly shutdown, a log entry is created and the exception is squashed. This ensures we don't unnecessarily crash systems that are already in the process of shutting down.

Fixes #38

If the worker is not cleanly shutdown, a log entry is created and the exception is squashed. This ensures we don't unnecessarily crash systems that are already in the process of shutting down.
@clintonb
Copy link
Contributor Author

clintonb commented Jun 9, 2023

@KurtzL

@clintonb
Copy link
Contributor Author

clintonb commented Jun 9, 2023

Actually...the shutdown isn't necessary at all. Temporal already listens for shutdown signals: https://typescript.temporal.io/api/interfaces/worker.RuntimeOptions#shutdownsignals.

@clintonb
Copy link
Contributor Author

clintonb commented Jun 9, 2023

I'm also seeing a client shutdown error during testing:

export function assignOnAppShutdownHook(client: WorkflowClient) {
(client as unknown as OnApplicationShutdown).onApplicationShutdown = client.connection.close;
return client;
}

@clintonb clintonb marked this pull request as draft June 9, 2023 17:58
@clintonb clintonb marked this pull request as ready for review June 21, 2023 16:13
@clintonb
Copy link
Contributor Author

Confirmed that client close is the source of my problems. This fixes them.

@clintonb clintonb force-pushed the clintonb/fix-shutdown branch from c4f7168 to 35076bb Compare June 22, 2023 15:47
@clintonb clintonb changed the title Squashing worker shutdown errors Squashing client and worker shutdown errors Jul 21, 2023
@clintonb
Copy link
Contributor Author

@KurtzL is there a timeline for reviewing and merging?

@KurtzL KurtzL self-assigned this Aug 13, 2023
@KurtzL KurtzL merged commit 83f2929 into KurtzL:main Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Squash worker shutdown errors
2 participants