feat: expose bundle retry settings #268
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #268 +/- ##
===========================================
- Coverage 71.75% 71.59% -0.16%
===========================================
Files 37 37
Lines 9280 9330 +50
Branches 537 537
===========================================
+ Hits 6659 6680 +21
- Misses 2617 2646 +29
Partials 4 4 ☔ View full report in Codecov by Sentry. |
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
error @permaweb/[email protected]: The engine "yarn" is incompatible with this module. Expected version "please-use-npm". Got "1.22.22" 📝 WalkthroughWalkthroughThis pull request introduces new configuration options for an AR.IO node by adding environment variables and updating corresponding configuration files. The changes focus on three main areas: trusted gateway request timeouts, bundle repair retry intervals, and bundle repair batch sizes. These modifications allow for more granular control over gateway request handling and bundle repair processes by enabling external configuration through environment variables with sensible default values. Changes
Possibly related PRs
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/envs.md (1)
79-80: Enhance documentation for bundle repair settings.The descriptions for the bundle repair settings could be more detailed to help users understand their impact.
Consider updating the descriptions to:
-| BUNDLE_REPAIR_RETRY_INTERVAL_SECONDS | String | "300" | Interval in seconds for retrying bundles | -| BUNDLE_REPAIR_RETRY_BATCH_SIZE | String | "1000" | Batch size for retrying bundles | +| BUNDLE_REPAIR_RETRY_INTERVAL_SECONDS | String | "300" | Time interval in seconds between attempts to repair failed bundle processing | +| BUNDLE_REPAIR_RETRY_BATCH_SIZE | String | "1000" | Maximum number of failed bundles to process in each repair attempt |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docker-compose.yaml(2 hunks)docs/envs.md(2 hunks)src/config.ts(2 hunks)src/data/gateways-data-source.test.ts(4 hunks)src/data/gateways-data-source.ts(3 hunks)src/workers/bundle-repair-worker.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (macos-latest)
🔇 Additional comments (12)
src/workers/bundle-repair-worker.ts (3)
19-19: LGTM!The import statement is correctly added and follows the project's import style.
68-68: LGTM!The retry interval is correctly configured using the environment variable, with proper conversion from seconds to milliseconds.
106-106: LGTM!The batch size is correctly configured using the environment variable.
src/data/gateways-data-source.ts (2)
32-32: LGTM!The import statement is correctly added and follows the project's import style.
42-42: LGTM!The request timeout is correctly configured using the environment variable.
src/data/gateways-data-source.test.ts (3)
28-34: LGTM!The helper function correctly encapsulates common mock parameters and follows TypeScript conventions.
56-56: LGTM!The mock setup correctly uses the helper function.
138-138: LGTM!The mock setup in test cases correctly uses the helper function.
Also applies to: 176-176
src/config.ts (2)
90-93: LGTM!The gateway request timeout configuration is correctly added with a sensible default value of 10 seconds.
260-263: LGTM!The bundle repair retry interval configuration is correctly added with a sensible default value of 5 minutes.
docs/envs.md (1)
12-12: LGTM! Well-documented timeout setting.The new timeout setting is properly documented with a clear type, default value, and description.
docker-compose.yaml (1)
50-50: LGTM! Environment variables are properly configured.The new environment variables are:
- Added to the correct service
- Follow the established pattern for variable declaration
- Logically grouped with related settings
Also applies to: 121-122
| gatewayAxios.interceptors.request.use((config) => { | ||
| this.log.debug('Axios request initiated', { | ||
| url: config.url, | ||
| method: config.method, | ||
| headers: config.headers, | ||
| params: config.params, | ||
| timeout: config.timeout, | ||
| }); | ||
| return config; | ||
| }); | ||
|
|
||
| gatewayAxios.interceptors.response.use( | ||
| (response) => { | ||
| this.log.debug('Axios response received', { | ||
| url: response.config.url, | ||
| status: response.status, | ||
| headers: response.headers, | ||
| }); | ||
| return response; | ||
| }, | ||
| (error) => { | ||
| if (error.response) { | ||
| this.log.error('Axios response error', { | ||
| url: error.response.config.url, | ||
| status: error.response.status, | ||
| headers: error.response.headers, | ||
| }); | ||
| } else { | ||
| this.log.error('Axios network error', { | ||
| message: error.message, | ||
| }); | ||
| } | ||
| return Promise.reject(error); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve logging security and consistency.
The interceptors enhance observability but have some concerns:
- Avoid logging sensitive information in headers that might contain auth tokens.
- Use consistent log levels - consider using
debugfor successful responses andwarnfor errors.
Apply this diff to improve the logging implementation:
gatewayAxios.interceptors.request.use((config) => {
this.log.debug('Axios request initiated', {
url: config.url,
method: config.method,
- headers: config.headers,
params: config.params,
timeout: config.timeout,
});
return config;
});
gatewayAxios.interceptors.response.use(
(response) => {
this.log.debug('Axios response received', {
url: response.config.url,
status: response.status,
- headers: response.headers,
});
return response;
},
(error) => {
if (error.response) {
- this.log.error('Axios response error', {
+ this.log.warn('Axios response error', {
url: error.response.config.url,
status: error.response.status,
- headers: error.response.headers,
});
} else {
- this.log.error('Axios network error', {
+ this.log.warn('Axios network error', {
message: error.message,
});
}
return Promise.reject(error);
},
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| gatewayAxios.interceptors.request.use((config) => { | |
| this.log.debug('Axios request initiated', { | |
| url: config.url, | |
| method: config.method, | |
| headers: config.headers, | |
| params: config.params, | |
| timeout: config.timeout, | |
| }); | |
| return config; | |
| }); | |
| gatewayAxios.interceptors.response.use( | |
| (response) => { | |
| this.log.debug('Axios response received', { | |
| url: response.config.url, | |
| status: response.status, | |
| headers: response.headers, | |
| }); | |
| return response; | |
| }, | |
| (error) => { | |
| if (error.response) { | |
| this.log.error('Axios response error', { | |
| url: error.response.config.url, | |
| status: error.response.status, | |
| headers: error.response.headers, | |
| }); | |
| } else { | |
| this.log.error('Axios network error', { | |
| message: error.message, | |
| }); | |
| } | |
| return Promise.reject(error); | |
| }, | |
| ); | |
| gatewayAxios.interceptors.request.use((config) => { | |
| this.log.debug('Axios request initiated', { | |
| url: config.url, | |
| method: config.method, | |
| params: config.params, | |
| timeout: config.timeout, | |
| }); | |
| return config; | |
| }); | |
| gatewayAxios.interceptors.response.use( | |
| (response) => { | |
| this.log.debug('Axios response received', { | |
| url: response.config.url, | |
| status: response.status, | |
| }); | |
| return response; | |
| }, | |
| (error) => { | |
| if (error.response) { | |
| this.log.warn('Axios response error', { | |
| url: error.response.config.url, | |
| status: error.response.status, | |
| }); | |
| } else { | |
| this.log.warn('Axios network error', { | |
| message: error.message, | |
| }); | |
| } | |
| return Promise.reject(error); | |
| }, | |
| ); |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/config.ts (1)
265-268: Add a comment explaining the default batch size.Consider adding a comment to explain why 1000 was chosen as the default batch size, similar to how the retry interval is documented.
export const BUNDLE_REPAIR_RETRY_BATCH_SIZE = +env.varOrDefault( 'BUNDLE_REPAIR_RETRY_BATCH_SIZE', - '1000', + '1000', // Process 1000 items per batch to balance throughput and memory usage );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/config.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (macos-latest)
- GitHub Check: test (ubuntu-latest)
🔇 Additional comments (2)
src/config.ts (2)
90-93: LGTM! The timeout configuration looks good.The default timeout of 10 seconds is reasonable for HTTP requests to trusted gateways.
260-263: LGTM! Well-documented retry interval configuration.The default interval of 5 minutes is reasonable for retry operations, and the comment clearly explains the value.
No description provided.