Repository navigation
feat(core,errors,client): per-request abort/timeout options and timeo… - #341
Merged
Cjay-Cyber-2 merged 10 commits intoSep 30, 2026
Merged
Cjay-Cyber-2 merged 10 commits into
Cjay-Cyber-2 merged 10 commits into
Conversation
…ut error hierarchy (ASTROIDX556#80) Closes ASTROIDX556#80 Every resource method now forwards per-request `RequestOptionsExtras` (`signal`, `timeoutMs`, `retryable`, headers) to the HTTP client, and a deadline miss is raised as a first-class member of the SDK error hierarchy instead of a detached `Error` subclass. Error hierarchy - `AstroidTimeoutError` moves to `@astroid/errors` as a subclass of `NetworkError`, so `isAstroidError`, `err instanceof NetworkError`, `err.code`, `err.isRetryable`/`err.retryable` and `toJSON()` all work. `@astroid/core` re-exports it, keeping the published import path. - Adds the `isTimeoutError` guard and the `ApiErrorCode.REQUEST_TIMEOUT` code, mapped in `errorClassForCode` so server-reported timeouts and client-raised deadlines share one class. - Because a timeout carries no HTTP status it is classified as a transport failure, so it now participates in the existing backoff retry path instead of escaping the typed-error branch. Timeout config hardening - `normalizeTimeoutMs` coerces `0`, negative, `NaN` and `Infinity` to `DEFAULT_TIMEOUT_MS`, applied to both the resolved client config and the per-request override. A stray `timeout: 0` can no longer turn every request into an instant `AstroidTimeoutError`. Per-request options - Threads `RequestOptionsExtras` through every resource namespace (agent, analytics, auth, budget, notification, policy, transaction, wallet, webhook, ai) and the standalone `submitSignedTransaction` helper. Pagination helpers forward the options on every page fetch, so an `AbortSignal` cancels mid-iteration instead of only page one. - `PolicyResource.simulateTransaction` takes a second `requestOptions` argument because its first parameter is already an options object. Validation: pnpm build, pnpm typecheck (17/17), pnpm lint (0 errors) and pnpm test (1,718 passed, 0 failed) all pass.
|
@Adebisi-1 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…e() alias The merge of main introduced a two-argument simulatePolicy overload (rule, transaction) on PolicyResource. The deprecated simulate() alias called this.simulatePolicy(input, options), which TypeScript resolved against the (PolicyRule, TransactionDetails) overload and failed (TS2345: PolicySimulationRequest not assignable to PolicyRule). Call the simulatePolicy endpoint helper directly so request options are still forwarded as intended.
Contributor
CI Checks Failed — Action NeededThe continuous integration checks on this pull request are failing. I can't merge until all required checks pass. Failing checks:
What to do: review the failing checks in the "Checks" tab above, fix the issues in your code, and push the changes. |
The merge of main into this branch left AstroidTimeoutError listed twice
in the same `import { ... } from './classes.js'` block — once after
AuthenticationError and once again after NetworkError. TypeScript
rejects that with TS2300 "Duplicate identifier", so the @astroid/errors
dts build failed and took the whole `build` job down with it. The
re-export list at the top of the file already exposes the class.
Also fix the import path in timeout-error-hierarchy.test.ts, which
imported '../src/index.js' from src/__tests__/ (resolving to the
nonexistent src/src/index.js) and left the fixture params untyped. Both
blocked `pnpm typecheck`, which never ran because it needs `build`.
Adebisi-1
added a commit
to Adebisi-1/astroid-sdk
that referenced
this pull request
Sep 30, 2026
…te and AgentResource.update PR ASTROIDX556#341 build failed with TS2554 in @astroid/policy: createPolicy() forwards `options` to create(), but create() only accepted the payload, so the dts build errored out. create() now takes the same RequestOptionsExtras every other method on the resource takes and passes it to the HTTP client. AgentResource.update() had the same gap and its tests already asserted the options argument, so those six tests were failing on the branch. It now accepts and forwards options too, and the deactivate/null-primaryWalletId assertions were updated to the three-argument call shape the implementation uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…te and AgentResource.update PR ASTROIDX556#341 build failed with TS2554 in @astroid/policy: createPolicy() forwards `options` to create(), but create() only accepted the payload, so the dts build errored out. create() now takes the same RequestOptionsExtras every other method on the resource takes and passes it to the HTTP client. AgentResource.update() had the same gap and its tests already asserted the options argument, so those six tests were failing on the branch. It now accepts and forwards options too, and the deactivate/null-primaryWalletId assertions were updated to the three-argument call shape the implementation uses.
Adebisi-1
force-pushed
the
feat/client-request-timeout-abortcontroller
branch
from
September 30, 2026 00:13
0c122f7 to
9b1dce3
Compare
….update and simulatePolicy PR ASTROIDX556#341's build job failed with three TS errors from the same root cause: the per-request abort/timeout work threaded an `options` argument through call sites whose resource methods still had the older, narrower signatures. - BudgetResource.get() now accepts `options?: RequestOptionsExtras` and forwards it to `getData(path, undefined, options)`, so validateBudgetSpend()'s `this.get(budgetId, requestOptions)` compiles and the caller's signal/timeout actually reach the HTTP client (TS2554). - PolicyResource.update() now accepts and forwards `options` to `client.patch()`, matching what its `updatePolicy()` alias already forwarded (TS2554). - The standalone simulatePolicy() endpoint helper forwards its `options` argument to `client.post()` instead of dropping it, which also clears the unused-parameter error (TS6133). These match the shape already used by the sibling methods fixed in 9b1dce3 and by wallet/notification/webhook, so the aliases stay consistent with their targets. pnpm build, pnpm typecheck and pnpm lint all pass; @astroid/budget (145) and @astroid/policy (289) test suites pass.
Cjay-Cyber-2
added a commit
that referenced
this pull request
Oct 7, 2026
…DTOs (#350) * feat(core,errors,client): per-request abort/timeout options and timeout error hierarchy (#80) Closes #80 Every resource method now forwards per-request `RequestOptionsExtras` (`signal`, `timeoutMs`, `retryable`, headers) to the HTTP client, and a deadline miss is raised as a first-class member of the SDK error hierarchy instead of a detached `Error` subclass. Error hierarchy - `AstroidTimeoutError` moves to `@astroid/errors` as a subclass of `NetworkError`, so `isAstroidError`, `err instanceof NetworkError`, `err.code`, `err.isRetryable`/`err.retryable` and `toJSON()` all work. `@astroid/core` re-exports it, keeping the published import path. - Adds the `isTimeoutError` guard and the `ApiErrorCode.REQUEST_TIMEOUT` code, mapped in `errorClassForCode` so server-reported timeouts and client-raised deadlines share one class. - Because a timeout carries no HTTP status it is classified as a transport failure, so it now participates in the existing backoff retry path instead of escaping the typed-error branch. Timeout config hardening - `normalizeTimeoutMs` coerces `0`, negative, `NaN` and `Infinity` to `DEFAULT_TIMEOUT_MS`, applied to both the resolved client config and the per-request override. A stray `timeout: 0` can no longer turn every request into an instant `AstroidTimeoutError`. Per-request options - Threads `RequestOptionsExtras` through every resource namespace (agent, analytics, auth, budget, notification, policy, transaction, wallet, webhook, ai) and the standalone `submitSignedTransaction` helper. Pagination helpers forward the options on every page fetch, so an `AbortSignal` cancels mid-iteration instead of only page one. - `PolicyResource.simulateTransaction` takes a second `requestOptions` argument because its first parameter is already an options object. Validation: pnpm build, pnpm typecheck (17/17), pnpm lint (0 errors) and pnpm test (1,718 passed, 0 failed) all pass. * fix(policy): forward options to endpoint helper in deprecated simulate() alias The merge of main introduced a two-argument simulatePolicy overload (rule, transaction) on PolicyResource. The deprecated simulate() alias called this.simulatePolicy(input, options), which TypeScript resolved against the (PolicyRule, TransactionDetails) overload and failed (TS2345: PolicySimulationRequest not assignable to PolicyRule). Call the simulatePolicy endpoint helper directly so request options are still forwarded as intended. * feat(types,agent): strict runtime validation for agent create/update DTOs (#215) Add zod schemas to @astroid/types that validate CreateAgentDto and UpdateAgentDto at runtime without duplicating the DTO types: - StellarPublicKeySchema (StrKey G..., 56 chars, base32 + checksum) - DecimalAmountStringSchema (no exponent/NaN/Infinity, >= 0) - AgentMetadataSchema (all values scalar, max 16 keys) - AgentInitialBudgetSchema (amount + optional assetId) - CreateAgentDtoSchema / UpdateAgentDtoSchema - isValid*/validate* helpers returning ValidationResult Wire the @astroid/agent create/update validators to the same rules via public assertion guards (assertValidStellarPublicKey, assertValidAmountString, validateAgentMetadata) so both layers reject identical payloads with AstroidValidationError {field, received, expected}. Tests: +32 in @astroid/types, +14 in @astroid/agent, including a 45-payload cross-layer agreement fixture table. * fix(client): correct timeout-error-hierarchy test imports The test imported ../src/index.js, which resolves to packages/client/src/src/index.js and cannot exist, and imported isTimeoutError from ../index.js, which does not re-export it, so the workspace typecheck job failed on main (base commit 80da5e7). Import isTimeoutError from @astroid/errors and type the fetch stub parameters from Parameters<typeof fetch>. Test-only change; no production code touched. * fix(errors,client): drop duplicate AstroidTimeoutError import The merge of main into this branch left AstroidTimeoutError listed twice in the same `import { ... } from './classes.js'` block — once after AuthenticationError and once again after NetworkError. TypeScript rejects that with TS2300 "Duplicate identifier", so the @astroid/errors dts build failed and took the whole `build` job down with it. The re-export list at the top of the file already exposes the class. Also fix the import path in timeout-error-hierarchy.test.ts, which imported '../src/index.js' from src/__tests__/ (resolving to the nonexistent src/src/index.js) and left the fixture params untyped. Both blocked `pnpm typecheck`, which never ran because it needs `build`. * fix(policy,agent): forward per-request options in PolicyResource.create and AgentResource.update PR #341 build failed with TS2554 in @astroid/policy: createPolicy() forwards `options` to create(), but create() only accepted the payload, so the dts build errored out. create() now takes the same RequestOptionsExtras every other method on the resource takes and passes it to the HTTP client. AgentResource.update() had the same gap and its tests already asserted the options argument, so those six tests were failing on the branch. It now accepts and forwards options too, and the deactivate/null-primaryWalletId assertions were updated to the three-argument call shape the implementation uses. * Fix README formatting by adding missing newline * feat(types,agent): strict runtime validation for agent create/update DTOs (#215) Add zod schemas to @astroid/types that validate CreateAgentDto and UpdateAgentDto at runtime without duplicating the DTO types: - StellarPublicKeySchema (StrKey G..., 56 chars, base32 + checksum) - DecimalAmountStringSchema (no exponent/NaN/Infinity, >= 0) - AgentMetadataSchema (all values scalar, max 16 keys) - AgentInitialBudgetSchema (amount + optional assetId) - CreateAgentDtoSchema / UpdateAgentDtoSchema - isValid*/validate* helpers returning ValidationResult Wire the @astroid/agent create/update validators to the same rules via public assertion guards (assertValidStellarPublicKey, assertValidAmountString, validateAgentMetadata) so both layers reject identical payloads with AstroidValidationError {field, received, expected}. Tests: +32 in @astroid/types, +14 in @astroid/agent, including a 45-payload cross-layer agreement fixture table. * test(types): restore agent DTO validation coverage * feat(types,agent): strict runtime validation for agent create/update DTOs (#215) Add zod schemas to @astroid/types that validate CreateAgentDto and UpdateAgentDto at runtime without duplicating the DTO types: - StellarPublicKeySchema (StrKey G..., 56 chars, base32 + checksum) - DecimalAmountStringSchema (no exponent/NaN/Infinity, >= 0) - AgentMetadataSchema (all values scalar, max 16 keys) - AgentInitialBudgetSchema (amount + optional assetId) - CreateAgentDtoSchema / UpdateAgentDtoSchema - isValid*/validate* helpers returning ValidationResult Wire the @astroid/agent create/update validators to the same rules via public assertion guards (assertValidStellarPublicKey, assertValidAmountString, validateAgentMetadata) so both layers reject identical payloads with AstroidValidationError {field, received, expected}. Tests: +32 in @astroid/types, +14 in @astroid/agent, including a 45-payload cross-layer agreement fixture table. * test(types): restore agent DTO validation coverage * fix(agent): enforce 52-character address validation * fix(budget,policy): forward per-request options to budget.get, policy.update and simulatePolicy PR #341's build job failed with three TS errors from the same root cause: the per-request abort/timeout work threaded an `options` argument through call sites whose resource methods still had the older, narrower signatures. - BudgetResource.get() now accepts `options?: RequestOptionsExtras` and forwards it to `getData(path, undefined, options)`, so validateBudgetSpend()'s `this.get(budgetId, requestOptions)` compiles and the caller's signal/timeout actually reach the HTTP client (TS2554). - PolicyResource.update() now accepts and forwards `options` to `client.patch()`, matching what its `updatePolicy()` alias already forwarded (TS2554). - The standalone simulatePolicy() endpoint helper forwards its `options` argument to `client.post()` instead of dropping it, which also clears the unused-parameter error (TS6133). These match the shape already used by the sibling methods fixed in 9b1dce3 and by wallet/notification/webhook, so the aliases stay consistent with their targets. pnpm build, pnpm typecheck and pnpm lint all pass; @astroid/budget (145) and @astroid/policy (289) test suites pass. * feat(wallet): add wallet resource methods (#275) --------- Co-authored-by: Adebisi-1 <oseniadebisi59@gmail.com> Co-authored-by: NobleDev <olamilekans389@gmail.com> Co-authored-by: Chijioke Joseph <jmhlabeni@byupathway.edu> Co-authored-by: Astroid Dev <astroid@dev.local> Co-authored-by: Chijioke Joseph <chijiokejoseph2022@gmail.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
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
This PR adds configurable request timeouts and
AbortSignalsupport to@astroid/client.The HTTP client now supports:
ClientOptions.AbortSignal.clearTimeoutto prevent timers from remaining active after requests complete.Unit tests were added with mocked
fetchto cover timeout behavior and caller-driven cancellation.Issue
Closes #80
Problem
The existing HTTP client wraps
fetch, but requests could remain pending indefinitely when the underlying network operation did not resolve.For autonomous agents, worker processes, and high-frequency applications, an indefinitely pending request can stall execution loops and prevent the application from making progress.
This change introduces predictable timeout behavior while still allowing callers to manually cancel individual requests.
What Changed
1. Configurable Client Timeout
ClientOptionsnow accepts an optionaltimeoutvalue in milliseconds.Example:
When no timeout is explicitly provided, the client uses the default timeout:
This provides predictable request behavior without requiring every caller to configure a timeout manually.
2. Request Timeout Using AbortController
Requests are now protected by an
AbortControllerandsetTimeout.The timeout flow is:
If the configured timeout expires before the request completes:
This prevents requests from hanging indefinitely.
3. Caller-Provided AbortSignal
Individual API requests can now receive a caller-provided
AbortSignal.This allows consumers to cancel an in-flight request themselves without waiting for the configured timeout.
Example:
The caller's signal is supported at the individual request level while the client continues to enforce its configured timeout.
4. Timeout Error
When the configured timeout is exceeded, the client throws a dedicated timeout error derived from the existing core error hierarchy.
This allows consumers to distinguish a timeout from other request or API failures.
Conceptually:
The timeout error is only raised for requests that exceed the configured timeout.
5. Timer Cleanup
Timeout timers are cleared when the request completes or is otherwise terminated.
This is handled with:
This prevents completed requests from leaving unnecessary timers active and avoids resource leaks in long-running applications.
Tests Added
Unit tests mock
fetchso timeout and cancellation behavior can be tested deterministically without relying on real network requests.The test coverage verifies:
Timeout Behavior
Default Timeout
Caller Cancellation
AbortSignalis passed to the underlying request.Timer Cleanup
API Behavior
The intended behavior is:
This gives callers both automatic protection against hanging requests and explicit control over cancellation.
Backward Compatibility
The timeout configuration is optional.
Existing consumers that create the client without specifying a timeout continue to work, with the new default timeout applied automatically.
Existing request behavior is otherwise preserved unless a request exceeds the configured timeout or is explicitly cancelled by the caller.
Files Changed
The implementation primarily affects:
Additional test files are included for the timeout and cancellation behavior.
Validation
Client Unit Tests
The client test suite was run with:
Result:
Type Checking
Type checking was run with:
cd packages/client pnpm typecheckResult:
Workspace Checks
Where applicable, the relevant workspace checks were also run to ensure the new client configuration and request options do not introduce TypeScript or test regressions.
Verification Checklist
timeoutAbortControllertimeout handlingsetTimeoutrequest cancellationAbortSignalsupportclearTimeoutfetchunit testspnpm testpassespnpm typecheckpassesRisk Assessment
Medium risk.
This change affects the shared HTTP request path in
@astroid/client, so timeout and cancellation behavior can affect all API methods using the client.The implementation is intentionally scoped to request lifecycle management:
The added unit tests provide coverage around the new request lifecycle behavior.
Design Considerations
Why
AbortController?fetchsupports cancellation throughAbortSignal. UsingAbortControllerallows the client to terminate the underlying request when the configured timeout expires rather than merely stopping the caller from waiting for the response.Why a Default Timeout?
Without a default, callers that do not configure a timeout could still experience indefinitely pending requests.
A 30-second default provides a predictable upper bound while allowing applications with different requirements to configure their own value.
Why Support Caller Signals?
Automatic timeouts and manual cancellation solve different problems.
The timeout protects against requests that remain pending unexpectedly, while a caller-provided signal allows application-level logic to cancel work intentionally—for example, when an agent task is stopped or a higher-level operation is no longer needed.
Final Verification
This PR adds request lifecycle controls to
@astroid/clientwithout changing the underlying API behavior.The client now provides:
The implementation is focused on predictable and controllable HTTP request behavior for long-running clients, autonomous agents, and high-frequency applications.