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

code-splitting: incorrect handling of import modules as not found when encountering temporary error #2326

Closed
prasannavl opened this issue Nov 26, 2018 · 6 comments · Fixed by #2400

Comments

@prasannavl
Copy link

prasannavl commented Nov 26, 2018

When a module is imported say with import("x") -- If the import fails due to a network failure. Parcel permanently seems to throw the same error that was received (in this case ERR_INTERNET_DISCONNECTED and reports module as MODULE_NOT_FOUND. This is incorrect. Parcel should always discard module load errors, so any fresh request can succeed.

PS: Webpack handles it correctly as described as well.

@await-ovo
Copy link

I'd like to fix this issue, and in my opinion just delete bundles[bundle]= can send fresh request, but i don't know if this will cause side effects . could someone help me?

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Dec 8, 2018

@diem98 not sure that's a good idea. As bundles[bundle] prevents loading bundles more than once (which is quite important if you'd load it in 20 diff places at the same time), and as far as I know it also contains the bundle, which isn't just true as you did.
Perhaps you could catch the error and do delete bundles[bundle] in that catch clause (and possible try to load the bundle again after a couple 100 ms?)

@await-ovo
Copy link

Thanks for your detailed explaining, but i wonder if we really need retry load bundle. if do the retry, then should try how many times or just one time? I think maybe just throw error as expected will enough.:)

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Dec 11, 2018

@diem98 What about changing the code to this:

if (bundleLoader) {
  return bundles[bundle] = bundleLoader(getBundleURL() + bundle)
    .then(function (resolved) {
      if (resolved) {
        module.bundle.register(id, resolved);
      }
 
      return resolved;
    })
    .catch(function (e) {
      delete bundles[bundle];
      // re-throw to be caught by the user
      throw e;
    });
}

*Haven't tested the code yet, so it's just an idea

and leaving the retry behaviour as future work?

@prasannavl
Copy link
Author

prasannavl commented Dec 11, 2018

@DeMoorJasper that looks good. However, I think it would be nicer to throw a custom error type, so that it can be caught specifically and handled by user code, for network failure.

And I don't see why retry has to be a part of this at all. The user code can always do the import that failed again if needed. I think retry introduces unnecessary magic and causes more problems than it solves. Without it, it's simple and deterministic - Either fails, or succeeds. Failure with a proper error type can be caught in user code and retried depending on the need of the application. (which is also consistent with webpack behavior).

@DeMoorJasper
Copy link
Member

@prasannavl alright sounds good, I've opened up a PR would be great if you could test it out to see if it does what you expect it to do.

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

Successfully merging a pull request may close this issue.

3 participants