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.

Sign up for GitHub

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

improvement CLDSRV-466 add timestamp for exceptions #5406

Merged
merged 1 commit into from
Nov 13, 2023
Merged

improvement CLDSRV-466 add timestamp for exceptions #5406

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
'use strict'; // eslint-disable-line strict

/**
* Catch uncaught exceptions and add timestamp to aid debugging
*/
process.on('uncaughtException', err => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to also catch the rejections?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's been a long time but this does catch rejections as well

process.stderr.write(`${new Date().toISOString()}: Uncaught exception: \n${err.stack}`);
});
Comment on lines +4 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

This will change the service behavior in case of error right? Like, we won't crash, and we won't end the current request either, is that what we want? Maybe I'm missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I thought too, but from observation he worker still exits after the message is printed.

Copy link
Contributor

Choose a reason for hiding this comment

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

old PR, but the worker exits because he adds its own listener (

cloudserver/lib/server.js

Lines 82 to 91 in 4f7aa54

process.on('uncaughtException', err => {
// If just send the error object results in empty
// object on server log.
logger.fatal('caught error', {
error: err.message,
stack: err.stack,
workerId: this.worker ? this.worker.id : undefined,
workerPid: this.worker ? this.worker.process.pid : undefined,
});
this.caughtExceptionShutdown();
) to shutdown after this one.

However the primary (that uses fork and has the monitoring server for prom metrics) don't have another listener, so this is keep the primary up in undefined state if an exception is caught


require('./lib/server.js')();
Loading