Skip to content

feat: allow modifying trustless-gateway fetch #751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 10, 2025

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Feb 27, 2025

  • feat: add support for custom headers in trustless gateway
  • chore: allow test trustless gateway server introspection
  • feat: allow modification of fetch options to trustless gateway

Title

feat: allow modifying trustless-gateway fetch

Description

This PR allows consumers to modify headers and any other RequestInit options that trustless-gateways use when making a fetch request to a trustless gateway.

This allows consumers to add JWT tokens to the request, or modifying the request in a variety of other ways. If the user does not modify the requestInit object, the default behavior is to use the same requestInit object that was used prior to this PR.

Fixes #659
Replaces #732

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

pcfreak30 and others added 3 commits February 1, 2025 04:48
Added an optional `headers` property to the `TrustlessGatewayBlockBroker` and `TrustlessGatewaySession` options, allowing users to specify custom headers for HTTP requests.

Changes include:
* Added `headers` property to `TrustlessGatewayBlockBroker` and `TrustlessGatewaySession` options
* Updated `findHttpGatewayProviders` function to accept an optional `headers` object
* Passed `headers` object to `TrustlessGateway` constructor
* Updated `TrustlessGateway` constructor to accept an optional `headers` object
* Added `headers` to HTTP request options in `TrustlessGateway`
@SgtPooki SgtPooki requested a review from a team as a code owner February 27, 2025 20:59
Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review

@SgtPooki
Copy link
Member Author

SgtPooki commented Feb 27, 2025

@pcfreak30 can you confirm this will work for your use-case?

cc @achingbrain

@SgtPooki SgtPooki requested a review from achingbrain February 27, 2025 21:05
@pcfreak30
Copy link
Contributor

@pcfreak30 can you confirm this will work for your use-case?

cc @achingbrain

At a glance, yes. primary use case is a JWT.

@SgtPooki
Copy link
Member Author

SgtPooki commented Feb 27, 2025

I'll need to look further into the chrome failures later. I was able to reproduce these locally but cannot any longer.

fixed with cd1f716 (#751)

@SgtPooki SgtPooki requested a review from 2color March 3, 2025 23:31
@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 4, 2025

ping @achingbrain and @2color for review

Copy link
Member

@2color 2color left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM.

I'd add an example to the helia README (not a blocker. we can do it in a followup)

@pcfreak30
Copy link
Contributor

Happy to see this get merged in :).

Thanks for collaborating.

@@ -33,7 +33,7 @@ export function filterNonHTTPMultiaddrs (multiaddrs: Multiaddr[], allowInsecure:
})
}

export async function * findHttpGatewayProviders (cid: CID, routing: Routing, logger: ComponentLogger, allowInsecure: boolean, allowLocal: boolean, options?: AbortOptions): AsyncGenerator<TrustlessGateway> {
export async function * findHttpGatewayProviders (cid: CID, routing: Routing, logger: ComponentLogger, allowInsecure: boolean, allowLocal: boolean, transformRequestInit?: TransformRequestInit, options: AbortOptions = {}): AsyncGenerator<TrustlessGateway> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that it's optional, can transformRequestInit go in the options object?

Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Small nit about the optional arg.

@SgtPooki SgtPooki merged commit 15de32f into main Mar 10, 2025
18 checks passed
@SgtPooki SgtPooki deleted the feat/allow-modifying-trustless-gateway-fetch-options branch March 10, 2025 21:20
@achingbrain achingbrain mentioned this pull request Mar 10, 2025
2color added a commit that referenced this pull request Mar 13, 2025
* origin/main:
  chore: use peer id parsing function from libp2p (#762)
  feat: add republish signed ipns records (#745)
  fix: use bytestream methods to add byte streams (#758)
  chore: bump codecov/codecov-action from 5.3.1 to 5.4.0 (#752)
  feat: allow modifying trustless-gateway fetch (#751)
  fix: align implicit default ttl with specs (#749)
  docs: add spell checker to ci (#743)
  chore: Update FUNDING.json for Optimism RPF (#746)
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.

@helia/block-brokers should support http headers being passed
4 participants