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

Handle application-level/ipfs-based deletions #241

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Ullaakut
Copy link
Contributor

@Ullaakut Ullaakut commented Aug 31, 2022

Goal of this PR

Fixes #228
Fixes #230

  • Makes the metadata fetcher return the status code of its requests
  • Makes the addition worker look at the error field within its payload when it receives an Internal Server Error or Not Found Error from the fetcher
  • Makes the addition stage use the sentinel error for ErrTokenNotFound to look for application-level/ipfs-based deleted tokens.

Draft

This PR remains a draft for now because it might have side effects from the fact that the lambda worker no longer fails for any status code that is different to 200. It now handles the case of 500 separately, and does not error if it's just a 500 that is not related to an app-level deletion, so should it still fail just with a different error? @awfm9

@Ullaakut Ullaakut requested a review from awfm9 August 31, 2022 13:37
@Ullaakut Ullaakut self-assigned this Aug 31, 2022
@Ullaakut Ullaakut changed the title Handle application-level deletions Handle application-level/ipfs-based deletions Aug 31, 2022
if err != nil {
return nil, fmt.Errorf("could not fetch remote metadata: %w", err)
}
switch code {
case http.StatusInternalServerError, http.StatusNotFound:
Copy link
Contributor

Choose a reason for hiding this comment

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

Those are two different cases, they should be handled in separate clauses.

  1. 500 Internal Server Error has a JSON error object with the TokenNot Found message;
  2. 404 Not Found has a simply return string in the body, that contains the IPFS missing message.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should probably be something like:

if code == http.StatusInternalServerError && isTokenNotFound(body) {
  return nil, ErrTokenNotFound
}
if code == http.StatusNotFound && isIPFSMissingLink(body) {
  return nil, ErrTokenNotFound
}

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

Successfully merging this pull request may close these issues.

Add support for IPFS-based deletions Add support for application level deletions
2 participants