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

CordovaAppLoader.js - line 173: consider reject() instead of resolve(null) #37

Closed
arieljake opened this issue Apr 12, 2015 · 2 comments
Closed

Comments

@arieljake
Copy link
Contributor

What do you think of

reject("New manifest available, but an earlier update attempt failed. Will not download."); 

instead of

resolve(null)

?

@markmarijnissen
Copy link
Owner

Interesting.

First thought: I expect check() to resolve when it successfully downloads the manifest.

Second thought: It makes sense to reject the manifest in order to tell the outside world: Please re-check the manifest (until it is a new manifest it can try).

Third thought: Re-checking the manifest (outside of the normal schedule) will not help resolve this problem.

The deeper problem is - there are many results of an check():

  • Valid response:
    • true: There is a new Manifest available
    • false: There is no new Manifest available.
    • null: There is a new manifest available, but a previous update attempt failed.
  • Error conditions:
    • XHR failed for server Manifest (i.e. internet connnection problem)
    • XHR failed for bundled Manifest
    • Cache failed to list files (filesystem errors)
    • Manifest is missing files attribute.

My current logic is:

  • on reject(), you should check() again.
  • on resolve(), you should deal with the results.
    • when true, you may download and update
    • when false, you can schedule another check later (or do nothing)
    • when null, you can schedule another check later (perhaps a bit faster), or you could reinstall the entire app (i.e. clear cache).

By the way, as a hack, you could clear last_update_files from localStorage after calling download() to ignore this protective measure. I could also make this an option.

@arieljake
Copy link
Contributor Author

Totally understand your thinking. Sounds good.

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

No branches or pull requests

2 participants