Skip to content
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

feat: fetch HTTP retrievals directly #82

Merged
merged 8 commits into from
Jul 10, 2024
Merged

feat: fetch HTTP retrievals directly #82

merged 8 commits into from
Jul 10, 2024

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Jul 9, 2024

When we're retrieving the content using the Trustless HTTP Gateway protocol ("http"), fetch the content directly from the provider, do not use Lassie.

This should give us better visibility into various error statuses returned by providers. For example, Lassie converts the error 429 Too Many Requests to a generic error 502 Bad Gateway.

List of synthetic status codes corresponding to different errors we may encounter along the new codepath:

  • 600 - unknown error (fallback)
  • 701 - provider's multiaddr has unsupported hostname type (e.g. ipv4 instead of ip4)
  • 702 - provider's multiaddr is not "tcp"
  • 703 - provider's multiaddr is not "https"
  • 704 - provider's multiaddr has too many parts
  • 801 - provider's hostname cannot be resolved via DNS
  • 802 - TCP connection error
  • 901 - CID uses an unsupported hash algorithm
  • 902 - payload's hash does not match the CID
  • 903 - provider returned unexpected blocks in the CAR response

TODO:

  • implementation
  • test coverage

Links:

When the retrieving the content using the Trustless HTTP Gateway
protocol ("http"), fetch the content directly from the provider,
do not use Lassie.

This should give us better visibility into various error statuses
returned by providers, e.g. 429 Too Many Requests, which Lassie converts
to generic 502 Bad Gateway error.

List of synthetic status codes corresponding to different errors we may
encounter along the new codepath:

- 900 - unknown error (fallback)
- 901 - provider's multiaddr is not "tcp"
- 902 - provider's multiaddr is not "https"
- 903 - provider's multiaddr has too many parts
- 911 - provider's hostname cannot be resolved via DNS
- 912 - TCP connection error
- 921 - CID uses an unsupported hash algorithm
- 922 - payload's hash does not match the CID
- 923 - provider returned unexpected blocks in the CAR response

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos requested a review from juliangruber July 9, 2024 11:54
Also: introduce a new synthetic status code 904 for unsupported host
types (e.g. `ipv4` instead of `ip4`).

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos force-pushed the feat-direct-http-fetch branch from eeddfb8 to 50326d3 Compare July 9, 2024 12:14
Signed-off-by: Miroslav Bajtoš <[email protected]>
Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

Great work Miro! 👏

bajtos added 2 commits July 9, 2024 15:15
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos force-pushed the feat-direct-http-fetch branch from a7210a4 to a573b58 Compare July 9, 2024 14:18
bajtos added a commit that referenced this pull request Jul 9, 2024
Run the tests for the code checking error messages that may be platform
specific, see #82.

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos mentioned this pull request Jul 9, 2024
bajtos added a commit that referenced this pull request Jul 9, 2024
Run the tests for the code checking error messages that may be platform
specific, see #82.

Signed-off-by: Miroslav Bajtoš <[email protected]>
bajtos added a commit that referenced this pull request Jul 9, 2024
Run the tests for the code checking error messages that may be platform
specific, see #82.

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos changed the title feat: direct fetch for HTTP retrievals feat: fetch HTTP retrievals directly Jul 9, 2024
@bajtos bajtos marked this pull request as ready for review July 9, 2024 15:38
@bajtos bajtos requested a review from juliangruber July 9, 2024 15:38
Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

great!!

@bajtos bajtos merged commit e23eda5 into main Jul 10, 2024
2 checks passed
@bajtos bajtos deleted the feat-direct-http-fetch branch July 10, 2024 08:06
Copy link

sentry-io bot commented Jul 11, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ExecaError: Command failed with exit code 1: /usr/src/app/modules/zinnia/zinniad spark/main.js ?(spark) View Issue
  • ‼️ ExecaError: Command failed with exit code 1: /core/modules/zinnia/zinniad spark/main.js ?(spark) View Issue
  • ‼️ ExecaError: Command failed with exit code 1: /core/modules/zinnia/zinniad spark/main.js ?(spark) View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants