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

Aggregator fall-through does not work on an HTTP 502 #343

Open
machawk1 opened this issue Jun 22, 2022 · 7 comments
Open

Aggregator fall-through does not work on an HTTP 502 #343

machawk1 opened this issue Jun 22, 2022 · 7 comments
Labels

Comments

@machawk1
Copy link
Owner

The default aggregator at memgator.cs.odu.edu is currently returning an HTTP 502. The logic for aggregator fall-through uses fetch, which will not invoke the catch to handle the error, thus the flow never resolves.

fetch should be returning a promise. We are returning window.fetch but somehow detection of this broken promise is not caught in the instance of this HTTP status code. Timely handling of this issue is needed before the default aggregator becomes alive again.

@machawk1
Copy link
Owner Author

Maybe because we are using the built built-in aborter in content.js?

return window.fetch(url, options)
  .then(setTimeout(() => { aborter.abort() }, timeout))
  .catch(error => {
    log(`${url} appears to be down, incrementing host counter`)
    log(error)
    hostI += 1
  })

@machawk1
Copy link
Owner Author

The 502 is likely occurring before the AbortController fires, hence no catch.

@machawk1
Copy link
Owner Author

There is nothing in the associated then (when added) that I am able to tell that distinguishes a successful response from a failed (502).

@machawk1
Copy link
Owner Author

XHR is not a solution here, as fetch will only be allowed in manifest v3. (#316)

@machawk1
Copy link
Owner Author

"Fetch detects only network errors. Other errors (4xx, 5xx) should be manually caught and rejected."

@machawk1
Copy link
Owner Author

The ODUCS aggregator is alive again and this issue is not yet fixed. A mock would be useful here for testing.

@machawk1 machawk1 removed the BLOCKER label Jul 1, 2022
machawk1 added a commit that referenced this issue Aug 24, 2023
Update closer to MV3 (#316)  by adapting fetch to catch 5XXs.
Improve flow of catching and calling, still not yet complete.
Re:#343 to better adapt fall-through logic, too.
@machawk1
Copy link
Owner Author

See #316 for some additional work on this front in adapting the extension to MV3.

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

No branches or pull requests

1 participant