-
Notifications
You must be signed in to change notification settings - Fork 1
Fix browser logging fixes #296
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
Changes from all commits
fc8b991
5fc183a
92fd01f
7b05f6e
165588e
0e82418
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,15 @@ export const loggerPrefix = '[Eppo SDK]'; | |
|
||
// Create a Pino logger instance | ||
export const logger = pino({ | ||
// eslint-disable-next-line no-restricted-globals | ||
level: process.env.LOG_LEVEL ?? (process.env.NODE_ENV === 'production' ? 'warn' : 'info'), | ||
// https://getpino.io/#/docs/browser | ||
browser: { disabled: true }, | ||
// Use any specified log level, or warn in production/browser, info otherwise | ||
/* eslint-disable no-restricted-globals */ | ||
level: | ||
typeof process !== 'undefined' && process.env.LOG_LEVEL | ||
? process.env.LOG_LEVEL | ||
: typeof process === 'undefined' || process.env.NODE_ENV === 'production' | ||
? 'warn' | ||
: 'info', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://getpino.io/#/docs/api?id=level-string Looks like it defaults to |
||
/* eslint-enable no-restricted-globals */ | ||
|
||
browser: { disabled: true }, // See https://getpino.io/#/docs/browser | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -756,14 +756,14 @@ export default class EppoClient { | |
|
||
try { | ||
this.logBanditAction(banditEvent); | ||
} catch (err: any) { | ||
logger.error('Error logging bandit event', err); | ||
} catch (err) { | ||
logger.error({ err }, 'Error logging bandit event'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pino's API (link) is to have context first, then the log message. |
||
} | ||
|
||
evaluationDetails.banditAction = action; | ||
} | ||
} catch (err: any) { | ||
logger.error('Error determining bandit action', err); | ||
logger.error({ err }, 'Error determining bandit action'); | ||
if (!this.isGracefulFailureMode) { | ||
throw err; | ||
} | ||
|
@@ -772,7 +772,7 @@ export default class EppoClient { | |
// Update the flag evaluation code to indicate that | ||
evaluationDetails.flagEvaluationCode = 'BANDIT_ERROR'; | ||
} | ||
evaluationDetails.flagEvaluationDescription = `Error evaluating bandit action: ${err.message}`; | ||
evaluationDetails.flagEvaluationDescription = `Error evaluating bandit action: ${err?.message}`; | ||
} | ||
return { variation, action, evaluationDetails }; | ||
} | ||
|
@@ -877,7 +877,7 @@ export default class EppoClient { | |
// Record in the assignment cache, if active, to deduplicate subsequent repeat assignments | ||
this.banditAssignmentCache?.set(banditAssignmentCacheProperties); | ||
} catch (err) { | ||
logger.warn('Error encountered logging bandit action', err); | ||
logger.warn({ err }, 'Error encountered logging bandit action'); | ||
} | ||
} | ||
|
||
|
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.
🔧 The fix: we now check that
process
is defined before accessing