-
Notifications
You must be signed in to change notification settings - Fork 10
fix: ACNA-4162 - add constructor options to set log level and logRetryAfterSeconds #102
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
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This will log a warning if the Retry-After header is greater than logRetryAfterSeconds. Set to 0 to disable.
| * @param {number} [options.logRetryAfterSeconds] the number of seconds after which to log a warning if the Retry-After header is greater than the number of seconds. Set to 0 to disable. | ||
| */ | ||
| constructor (options = {}) { | ||
| this.logLevel = options.logLevel || process.env.LOG_LEVEL || 'info' |
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.
why keep this property? looks like its only used to initialize this.logger
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
This PR adds constructor options to configure log level and retry-after warning thresholds for the networking library components, improving flexibility for dependent libraries in serverless environments.
Key Changes
- Added
logLevelconstructor option to bothHttpExponentialBackoffandProxyFetchclasses to allow programmatic log level configuration - Added
logRetryAfterSecondsoption toHttpExponentialBackoffto control when retry delays trigger warning vs debug logs - Renamed
ProxyAuthOptionstoProxyOptionsthroughout the codebase for better naming clarity
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| types.d.ts | Added constructor signatures with logLevel and logRetryAfterSeconds options; renamed ProxyAuthOptions to ProxyOptions |
| src/HttpExponentialBackoff.js | Implemented constructor with logLevel and logRetryAfterSeconds options; refactored internal functions to arrow functions for proper this binding |
| src/ProxyFetch.js | Added logLevel option to constructor and instance logger initialization; renamed parameters from proxyAuthOptions to proxyOptions |
| src/utils.js | Updated parameter naming from proxyAuthOptions to proxyOptions for consistency |
| test/HttpExponentialBackoff.test.js | Added comprehensive tests for Retry-After header handling and logRetryAfterSeconds functionality; added missing spy cleanup |
| README.md | Updated documentation to reflect new constructor options and renamed types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #101
Closes #99
Related: #102
Description
Currently the log level can be set by setting the env var
LOG_LEVELonly which may be less intuitive to set in serverless environments. Also a logRetryAfter setting is added - the number of seconds after which to log a warning if the Retry-After header is greater than the number of seconds. Set to 0 to disable.Allow dependent callers of this library to set the log level via their respective constructors - for example @adobe/aio-lib-state library.
Related Issue
See #101
How Has This Been Tested?
npm test
used this code:
Screenshots (if appropriate):
Types of changes
Checklist: