Skip to content

fix(api): treat post opts as request options, not headers#1868

Closed
kinderjoypresents wants to merge 1 commit intoProjectOpenSea:mainfrom
kinderjoypresents:fix/api-post-opts-vs-headers
Closed

fix(api): treat post opts as request options, not headers#1868
kinderjoypresents wants to merge 1 commit intoProjectOpenSea:mainfrom
kinderjoypresents:fix/api-post-opts-vs-headers

Conversation

@kinderjoypresents
Copy link
Contributor

Problem

OpenSeaAPI.post(apiPath, body, opts) documents opts as ConnectionInfo-like request options, but the internal _fetch previously treated the entire opts object as headers. This could accidentally send fields like timeout as HTTP headers.

Solution

Update _fetch to:

  • preserve backward compatibility when a plain object is passed as headers (e.g. { Authorization: "Bearer ..." })
  • support opts.headers for explicit header passing
  • apply only a small known-safe subset of request option keys (e.g. timeout, signal) and never forward them as headers

Tests

  • verifies timeout is not sent as a header
  • verifies plain header object remains supported (Authorization)

@ryanio
Copy link
Collaborator

ryanio commented Jan 15, 2026

Hey @kinderjoypresents, thanks for the PR and your others! Before we consider merging, could you share a concrete scenario where you hit this bug? The current behavior has been stable, and while I understand the docs/implementation mismatch you're pointing out, the heuristic-based detection (looksLikeHeaders) adds complexity that could introduce subtle bugs of its own.

If there's a real use case driving this, I'd prefer a simpler solution - either properly typing the options interface or just being explicit that the parameter only accepts headers. Let me know what problem you were trying to solve and we can discuss the right approach.

@kinderjoypresents
Copy link
Contributor Author

Hey @kinderjoypresents, thanks for the PR and your others! Before we consider merging, could you share a concrete scenario where you hit this bug? The current behavior has been stable, and while I understand the docs/implementation mismatch you're pointing out, the heuristic-based detection (looksLikeHeaders) adds complexity that could introduce subtle bugs of its own.

If there's a real use case driving this, I'd prefer a simpler solution - either properly typing the options interface or just being explicit that the parameter only accepts headers. Let me know what problem you were trying to solve and we can discuss the right approach.

Hey, thanks for the thoughtful feedback.

A concrete scenario where I hit this: I needed to pass request-level options to the underlying fetch/HTTP layer (not headers). For example:

  • Abort/timeout control via signal (AbortController) to avoid requests hanging indefinitely (common in CI, server-side batch scripts, or when OpenSea’s API is slow/intermittent).
  • Custom agent / proxy settings in Node (e.g. corporate proxy, keep-alive agent), which are also request options.

ryanio added a commit that referenced this pull request Jan 15, 2026
Add support for request-level options in post() method:
- timeout: Set request timeout in milliseconds
- signal: Pass AbortController signal for request cancellation

This enables:
- Abort/timeout control via AbortController (useful for CI, batch scripts)
- Custom timeout settings per request

The new RequestOptions interface is exported for SDK consumers.

Closes the use case from #1868 with a clean, typed interface instead
of heuristics.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ryanio
Copy link
Collaborator

ryanio commented Jan 15, 2026

Thanks for explaining the use case! That makes sense - timeout/abort control and custom agents are legitimate request-level concerns.

I've opened #1871 which adds a proper typed RequestOptions interface:

  await api.post('/path', body, headers, {
    timeout: 5000,           // timeout in ms
    signal: controller.signal // AbortController support
  });

This addresses your timeout/abort use case with explicit typing instead of heuristics. The interface is also exported so SDK consumers can use it.

For custom agent/proxy settings - those would need additional work since ethers.FetchRequest doesn't directly expose that. If that's important for your use case, let me know and we can explore options (might require a different approach like allowing a custom fetch implementation, but would prefer to stick to ethers for simplicity and relying on battle tested library than rolling our own).

@ryanio ryanio closed this Jan 15, 2026
ryanio added a commit that referenced this pull request Jan 15, 2026
* feat(api): add request options for timeout and abort signal

Add support for request-level options in post() method:
- timeout: Set request timeout in milliseconds
- signal: Pass AbortController signal for request cancellation

This enables:
- Abort/timeout control via AbortController (useful for CI, batch scripts)
- Custom timeout settings per request

The new RequestOptions interface is exported for SDK consumers.

Closes the use case from #1868 with a clean, typed interface instead
of heuristics.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* feat(api): add request options to get() method

Extend RequestOptions support to get() for consistency with post().
Same timeout and abort signal options now available for GET requests.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>
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.

2 participants