-
-
Notifications
You must be signed in to change notification settings - Fork 0
Adds option to exit process on logger error #402
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
Conversation
Introduces the `exitOnError` option to the logger configuration. This allows the logger to exit the process when an error is logged, providing a mechanism to halt execution in critical failure scenarios.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdded an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds an exitOnError option to the logger factory so callers can control Winston’s exitOnError behavior via this library’s createLogger wrapper.
Changes:
- Documented a new
options.exitOnErrorparameter in thecreateLoggerJSDoc. - Added
exitOnErrorto thecreateLoggeroptions destructuring with a default value. - Passed
exitOnErrorthrough towinston.createLogger(...).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/logger.js`:
- Around line 43-49: The logger config passes exitOnError to
winston.createLogger but you manually handle exceptions with
process.on('uncaughtException'), so either remove the exitOnError option from
the logger construction or enable Winston's exception handling instead; to fix,
update the winston.createLogger call (symbol: logger) by deleting the
exitOnError property if you want manual handling, or replace manual handling by
configuring a transport with handleExceptions: true or using exceptionHandlers
in the winston setup and remove the process.on('uncaughtException', ...) handler
accordingly.
| const logger = winston.createLogger({ | ||
| level, | ||
| levels, | ||
| defaultMeta: meta, | ||
| transports: winstonLoggers | ||
| transports: winstonLoggers, | ||
| exitOnError | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
winston 3 exitOnError option behavior
💡 Result:
In winston v3, exitOnError controls whether the Node.js process should exit after winston logs an uncaughtException (i.e., when you’ve enabled exception handling via handleExceptions: true on a transport or via exceptionHandlers). By default, winston will exit after logging the uncaught exception. [1][2]
You can change this behavior:
-
Never exit after an uncaught exception log:
const logger = winston.createLogger({ exitOnError: false }); // or later: logger.exitOnError = false;
[1][2]
-
Conditional exit (function form):
exitOnError(err)can decide per error whether to exit. Example shown in docs:function ignoreEpipe(err) { return err.code !== 'EPIPE'; } const logger = winston.createLogger({ exitOnError: ignoreEpipe });
(In this example, it won’t exit for
EPIPE, but will exit for other errors.) [1][2]
Note: exitOnError is specifically documented in the context of uncaught exception handling, not normal logger.error(...) calls. [1][2]
Sources: [1] [2]
🏁 Script executed:
# Find and examine the logger.js file to understand context
find . -name "logger.js" -type fRepository: trojs/logger
Length of output: 157
🏁 Script executed:
# Examine the main logger.js file to understand the full context
cat -n ./src/logger.jsRepository: trojs/logger
Length of output: 4957
Remove unused exitOnError parameter or configure Winston exception handling.
The exitOnError option only takes effect when Winston is configured to handle uncaught exceptions (via handleExceptions: true on a transport or exceptionHandlers). This code manually handles exceptions with process.on('uncaughtException', ...) instead, making exitOnError ineffective. Either remove the parameter or switch to Winston's built-in exception handling if that's the intent.
🤖 Prompt for AI Agents
In `@src/logger.js` around lines 43 - 49, The logger config passes exitOnError to
winston.createLogger but you manually handle exceptions with
process.on('uncaughtException'), so either remove the exitOnError option from
the logger construction or enable Winston's exception handling instead; to fix,
update the winston.createLogger call (symbol: logger) by deleting the
exitOnError property if you want manual handling, or replace manual handling by
configuring a transport with handleExceptions: true or using exceptionHandlers
in the winston setup and remove the process.on('uncaughtException', ...) handler
accordingly.
Corrects the default `exitOnError` value for the logger. Ensures the logger does not exit on error by default, allowing for more flexible error handling.
Introduces the
exitOnErroroption to the logger configuration. This allows the logger to exit the process when an error is logged, providing a mechanism to halt execution in critical failure scenarios.Summary by CodeRabbit
New Features
exitOnErrorconfiguration to the logger to control error-exit behavior; defaults to false for backward compatibility.Tests