Skip to content

Centralize request routing, enforce feature parity in CI, add request cancellation#21

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/centralize-request-routing
Draft

Centralize request routing, enforce feature parity in CI, add request cancellation#21
Copilot wants to merge 2 commits intomainfrom
copilot/centralize-request-routing

Conversation

Copy link

Copilot AI commented Mar 1, 2026

Four API methods bypassed the centralized request()/_request() methods, duplicating auth/error handling and excluding themselves from future retry/observability hooks. The feature parity CI check always exited 0. The CI matrix only covered Python 3.14-pre and Node 20.

Centralized request routing

  • TS: getHistory, getAssetTree, searchAsset, searchNft now route through request() instead of calling fetch() directly. Body type widened to FormData | Record<string, unknown> | unknown[] to support array payloads (getAssetTree).
  • Python: get_history, get_asset_tree, search_asset, search_nft now route through _request(). json_body type widened to dict[str, Any] | list[Any] | None.

Request cancellation + custom fetch (TypeScript)

Two new optional fields on CaptureOptions applied to every request automatically:

const capture = new Capture({
  token: '...',
  signal: controller.signal,          // AbortSignal for cancellation
  fetchImplementation: customFetch,   // proxy / custom TLS / keep-alive
})

Feature parity CI enforcement

print_report() now returns bool; main() calls sys.exit(1) on violations.

CI matrix & Python compatibility

  • Python: 3.11 / 3.12 / 3.13 (was 3.14-only); requires-python lowered to >=3.11
  • Node.js: 18 / 20 / 22 (was 20-only)
  • --cov-fail-under=75 added to pytest
  • permissions: contents: read added to workflow
Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature][Medium] Centralize request routing, enforce feature parity in CI, and add request cancellation</issue_title>
<issue_description>## Feature Improvements — Medium Priority

1. Centralize All API Calls Through request() Method

Files:

  • ts/src/client.ts lines 363-396 (getHistory), 412-448 (getAssetTree)
  • python/numbersprotocol_capture/client.py lines 454-466, 519-523

Description:
The TypeScript Capture class has a centralized request() method (lines 158-194) that handles authentication, error parsing, and response deserialization. However, getHistory, getAssetTree, searchAsset, and searchNft all call fetch() directly, bypassing this method. The same pattern exists in Python.

This means any future enhancements (retry logic from issue #7, logging, metrics, request tracing) will not automatically apply to these methods. Error handling is also duplicated and slightly inconsistent.

Expected impact:
All API calls benefit from centralized error handling, future retry logic, and observability hooks. This is a prerequisite for cleanly implementing issue #7.

Suggested implementation:
Refactor request() to accept a full URL (not just a path relative to baseUrl), or create a _rawRequest() that handles external URLs. Route all external API calls through it.


2. Feature Parity Script Does Not Fail CI on Violations

File: scripts/check-feature-parity.py lines 212-219

Description:
The feature parity script prints a report but the main() function always exits with code 0 (success). It never calls sys.exit(1) when parity violations are detected. This means the CI workflow step will always pass even if one SDK is missing features.

Expected impact:
Actually enforces feature parity between TypeScript and Python SDKs in CI.

Suggested implementation:

def main() -> None:
    check_ts_features()
    check_py_features()
    all_parity = print_report()
    if not all_parity:
        sys.exit(1)

3. Add Request Cancellation and Custom HTTP Configuration for TypeScript

File: ts/src/client.ts lines 141-153, 176

Description:
The TypeScript SDK uses the global fetch function with no connection management. There is no way to:

  1. Use a custom HTTP agent (for proxies, custom TLS certificates, keep-alive tuning)
  2. Control connection pooling
  3. Cancel in-flight requests via AbortController

The request() method at line 176 passes no signal option to fetch.

Expected impact:
Enables request cancellation, proxy support, and custom HTTP configuration for enterprise users.

Suggested implementation:

  • Add optional signal?: AbortSignal parameter to CaptureOptions or individual method options.
  • Add optional fetchImplementation option to CaptureOptions for custom fetch implementations.
  • Pass signal through to fetch() in the request() method.

4. CI Version Matrix Is Too Narrow

File: .github/workflows/ci.yml lines 69-99

Description:

  • Python is only tested on 3.14 (pre-release). pyproject.toml specifies requires-python = ">=3.14" which excludes Python 3.10-3.13 users entirely. This is likely an error.
  • Node.js is only tested on version 20. The engines field says >=18.0.0, but Node 18 and 22 are not tested.
  • No test coverage thresholds are enforced in CI (pytest-cov is configured but --cov-fail-under is not set).

Expected impact:
Broader compatibility assurance, catches regressions earlier, prevents coverage decay.

Suggested implementation:

  • Lower requires-python to >=3.11 or >=3.12 and add those versions to the CI matrix.
  • Add Node.js 18 and 22 to a matrix strategy.
  • Add --cov-fail-under=80 to pytest configuration.</issue_description>

Comments on the Issue (you are @copilot in this section)


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

… support, expand CI matrix

Co-authored-by: numbers-official <181934381+numbers-official@users.noreply.github.com>
Copilot AI changed the title [WIP] Centralize request routing and improve error handling Centralize request routing, enforce feature parity in CI, add request cancellation Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature][Medium] Centralize request routing, enforce feature parity in CI, and add request cancellation

2 participants