feat: add options to disable abort/timeout mechanisms#638
Open
feat: add options to disable abort/timeout mechanisms#638
Conversation
Post-release cleanup for release 72: - Updated version to 73-pre - Reset core docker image tags to latest (observer/AO remain pinned) - Updated AR_IO_NODE_RELEASE to 73-pre - Added [Unreleased] section to CHANGELOG.md Development branch ready for release 73 features and fixes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…eways Add three new environment variables for selectively disabling abort and timeout behaviors: - TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS: disable connection timeout aborts - TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS: disable stream stall aborts - DISABLE_REQUEST_ABORT_SIGNAL: disable client-disconnect abort propagation Also fix unguarded clearTimeout on the success path in GatewaysDataSource for consistency with the error path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds three config flags to disable AbortController-based timeouts and abort propagation; wires them into config, docs, docker-compose, middleware, GatewaysDataSource, tests, and bumps the release to 73-pre. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/config.ts (1)
201-212: Document the new abort toggles inline.These are new operator-facing config exports, but unlike the surrounding settings they were added without inline docs. Please add short TSDoc/comments describing what each flag disables and its default behavior.
Suggested doc shape
+/** + * Disable connection-timeout aborts for trusted gateway requests. + * Defaults to false. + */ export const TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS = env.varOrDefault( 'TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS', 'false', ) === 'true'; +/** + * Disable stream-stall aborts for trusted gateway responses. + * Defaults to false. + */ export const TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS = env.varOrDefault('TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS', 'false') === 'true'; +/** + * Disable request abort propagation from client disconnects. + * Defaults to false. + */ export const DISABLE_REQUEST_ABORT_SIGNAL = env.varOrDefault('DISABLE_REQUEST_ABORT_SIGNAL', 'false') === 'true';As per coding guidelines:
**/*.{ts,tsx,js}: Add or improve TSDoc comments when modifying code to enhance documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 201 - 212, Add short TSDoc comments above the three new exports to document operator-facing behavior: for TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS explain it disables aborting trusted-gateway proxied requests when the upstream request timeout elapses and that it defaults to false (do not abort); for TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS explain it disables aborting trusted-gateway proxied streaming responses on perceived stream stalls and that it defaults to false (do not abort); for DISABLE_REQUEST_ABORT_SIGNAL explain it disables attaching/propagating an AbortSignal to outgoing requests (useful for legacy clients) and that it defaults to false. Include the constant names (TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS, TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS, DISABLE_REQUEST_ABORT_SIGNAL) in the comments and keep each TSDoc to one or two short sentences.src/middleware/abort-signal.ts (1)
9-24: Consider documenting the new parameter in TSDoc.The existing TSDoc describes the middleware's purpose but doesn't mention the
disableRequestAbortSignaloption. Adding a@paramentry would improve discoverability.📝 Suggested TSDoc enhancement
/** * Middleware that attaches an AbortSignal to each request. * The signal is aborted when the client disconnects before the response completes. * * Usage in handlers: * const signal = req.signal; * await someAsyncOperation({ signal }); * * This allows downstream operations to be cancelled when clients disconnect, * preventing wasted work on requests that will never be delivered. + * + * `@param` options.disableRequestAbortSignal - If true, the signal is not aborted + * when the client disconnects. Defaults to false. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middleware/abort-signal.ts` around lines 9 - 24, Add a `@param` entry to the TSDoc for createAbortSignalMiddleware documenting the disableRequestAbortSignal option: describe its type (boolean), default (false), and effect (when true the middleware will not attach/abort request signals). Reference the option name disableRequestAbortSignal and briefly show intended behavior (disable Request AbortSignal attachment/abortion) so callers can discover and understand the flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/config.test.ts`:
- Around line 96-137: Add assertions that the default values for
TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS and DISABLE_REQUEST_ABORT_SIGNAL
are false in the same "Trusted gateway timeout config" suite: when their env
vars are deleted (or unset) import ../src/config.js with the cache-busting query
and assert config.TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS === false and
config.DISABLE_REQUEST_ABORT_SIGNAL === false; keep these new checks alongside
the existing TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS default test so
regressions to a truthy default are caught.
---
Nitpick comments:
In `@src/config.ts`:
- Around line 201-212: Add short TSDoc comments above the three new exports to
document operator-facing behavior: for
TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS explain it disables aborting
trusted-gateway proxied requests when the upstream request timeout elapses and
that it defaults to false (do not abort); for
TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS explain it disables aborting
trusted-gateway proxied streaming responses on perceived stream stalls and that
it defaults to false (do not abort); for DISABLE_REQUEST_ABORT_SIGNAL explain it
disables attaching/propagating an AbortSignal to outgoing requests (useful for
legacy clients) and that it defaults to false. Include the constant names
(TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTS,
TRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTS, DISABLE_REQUEST_ABORT_SIGNAL) in
the comments and keep each TSDoc to one or two short sentences.
In `@src/middleware/abort-signal.ts`:
- Around line 9-24: Add a `@param` entry to the TSDoc for
createAbortSignalMiddleware documenting the disableRequestAbortSignal option:
describe its type (boolean), default (false), and effect (when true the
middleware will not attach/abort request signals). Reference the option name
disableRequestAbortSignal and briefly show intended behavior (disable Request
AbortSignal attachment/abortion) so callers can discover and understand the
flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6c316065-7b46-41dd-83a1-1c5b802c1645
📒 Files selected for processing (11)
CHANGELOG.mddocker-compose.yamldocs/envs.mdsrc/app.tssrc/config.tssrc/data/gateways-data-source.test.tssrc/data/gateways-data-source.tssrc/middleware/abort-signal.test.tssrc/middleware/abort-signal.tssrc/version.tstest/config.test.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #638 +/- ##
===========================================
+ Coverage 76.73% 76.78% +0.05%
===========================================
Files 105 106 +1
Lines 36346 36426 +80
Branches 2654 2663 +9
===========================================
+ Hits 27891 27971 +80
Misses 8421 8421
Partials 34 34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…nfig Address CodeRabbit PR feedback: add missing default-value test assertions for stream stall and request abort signal configs, add TSDoc comments on new config exports, and add @param documentation for middleware option. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TRUSTED_GATEWAYS_DISABLE_REQUEST_TIMEOUT_ABORTSto disable connection timeout aborts for trusted gateway requestsTRUSTED_GATEWAYS_DISABLE_STREAM_STALL_ABORTSto disable stream stall aborts for trusted gateway responsesDISABLE_REQUEST_ABORT_SIGNALto disable client-disconnect abort propagation globallyclearTimeouton the success path inGatewaysDataSourceTest plan
yarn test:file test/config.test.tsyarn test:file src/data/gateways-data-source.test.tsyarn test:file src/middleware/abort-signal.test.tsfalse)trueto confirm timeouts/aborts are properly disabled🤖 Generated with Claude Code