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

Add blocking error type for gateway usage #591

Open
aschmahmann opened this issue Mar 22, 2024 · 2 comments
Open

Add blocking error type for gateway usage #591

aschmahmann opened this issue Mar 22, 2024 · 2 comments
Labels
kind/enhancement A net-new feature or improvement to an existing feature need/triage Needs initial labeling and prioritization

Comments

@aschmahmann
Copy link
Contributor

Right now we don't use proper error types to convey that some content requested by a gateway was blocked. Instead we have this string based checking tied to nopfs

boxo/gateway/errors.go

Lines 211 to 216 in b101ba0

// isErrContentBlocked returns true for content filtering system errors
func isErrContentBlocked(err error) bool {
// TODO: we match error message to avoid pulling nopfs as a dependency
// Ref. https://github.com/ipfs-shipyard/nopfs/blob/cde3b5ba964c13e977f4a95f3bd8ca7d7710fbda/status.go#L87-L89
return strings.Contains(err.Error(), "blocked and cannot be provided")
}

@MichaelMure recently let me know that:

  1. Infura also does some string based error checking for their errors
  2. They return 451 when rather than just 410

So it seems to me like this should be bundled into boxo tooling.

  1. Have an error type indicating that it's a blocked error (which other tools like nopfs or others can wrap)
  2. Given that 451 and 410 both seem plausible either that should be configurable by the error tooling, or at the gateway level
    • At the moment I don't know anyone mixing 410s + 451s but that may not always be the case

@MichaelMure let me know if there's any context I missed.

cc @lidel

@aschmahmann aschmahmann added kind/enhancement A net-new feature or improvement to an existing feature need/triage Needs initial labeling and prioritization labels Mar 22, 2024
@MichaelMure
Copy link
Contributor

Note: the same concerns exist for https://github.com/ipfs/go-ipfs-cmds. If we are not able to also return adequate HTTP status code there, then we'd still have to do string matching.

As for 451 vs 410, it's not too important imho. 451 seems more appropriate to me but if kubo goes for 410, I think we'd align to that.

@MichaelMure
Copy link
Contributor

As for https://github.com/ipfs/go-ipfs-cmds, I think it would be a goal with beneficial side effects: right now, spread into kubo, some errors are just bubbling up and surface as 500s, because there is no way to classify them in a meaningful way for the user/client interface.

On the top of my head, there is:

  • 410/451: legal removal
  • 429: rate limiting
  • 504: timeout
  • (possibly) 413: content too large
  • (possibly) 507: insufficient storage

Typically, those kind of errors would be implemented in a proxy, but that remains awkward. As more and more logic get built and integrated into kubo (or on top of boxo), more and more sources of this kind of errors get triggered deep into kubo/boxo, with no way to interpret them besides string matching. We already have errors akin to 451, 504 and 507 in those places.

A set of error wrapper, interpreted by the gateway and https://github.com/ipfs/go-ipfs-cmds would help quite a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

2 participants