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

Refactor blob load related error handling in multiple places. #9

Closed
wants to merge 1 commit into from

Conversation

knadh
Copy link
Owner

@knadh knadh commented Sep 9, 2021

  • Use Promise.allSettled() to capture failures.
  • Refactor _applyOriginal() and _applyBlob() to a single
    method _applyElement() that fails over the element to the
    original URL if the blob is null.
  • Make _getObject() return a dummy data{} object with null blob
    instead of a reject() in case of failed DB+HTTP fetch so that
    _applyElement() gracefully fails over to the original URL.
  • Rename _getBlob() to _getDBblob() for clarity.
  • Add <script>.onerror() to not break the load chain in case
    a script fails to load.

- Use `Promise.allSettled()` to capture failures.
- Refactor `_applyOriginal()` and `_applyBlob()` to a single
  method `_applyElement()` that fails over the element to the
  original URL if the blob is null.
- Make `_getObject()` return a dummy data{} object with null blob
  instead of a reject() in case of failed DB+HTTP fetch so that
  `_applyElement()` gracefully fails over to the original URL.
- Rename `_getBlob()` to `_getDBblob()` for clarity.
- Add `<script>.onerror()` to not break the load chain in case
  a script fails to load.
@ketan-10
Copy link
Contributor

ketan-10 commented Sep 9, 2021

Great!
This PR resolves the issue #7
Also dissolved PR #8

// Get the object from the DB and if that fails, fetch() it over HTTP
// This function should not reject a promise and in the case of failure,
// will return a dummy data object as if it were fetched from the DB.
async _getObject (obj) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see few methods are marked as async even though no await is not used, is there any reason?

Copy link
Owner Author

Choose a reason for hiding this comment

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

They needn't be anymore. Will clean up in this branch.

@knadh
Copy link
Owner Author

knadh commented Sep 9, 2021

WIP. Strange issues on Safari continue to persist.

@knadh
Copy link
Owner Author

knadh commented Sep 9, 2021

Fixed in fa7a650

@knadh knadh closed this Sep 9, 2021
@vividvilla vividvilla deleted the handle-error branch September 17, 2021 04:40
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.

2 participants