Loadable-components: Implement retry in case of error

Created on 7 Dec 2020  ·  9Comments  ·  Source: gregberge/loadable-components

🚀 Feature Proposal

Implement a new "retry" option in @loadable/component.

Motivation

We monitor our errors with Sentry and we see quite a lot of errors coming with lazy-loaded components. Indeed, on flaky network, it's easy to have a request aborted and the component is never loaded. In our applications, we have started to wrap all dynamic imports passed to loadable with a retry function such as:

const Example = loadable(() =>
  retry(
    () => import('./Example'),
    { retries: 3 }
  )
)

where retry is a simple function:

export function retry(
  fn: () => Promise<any>,
  { retries = 3, interval = 500, exponentialBackoff = true }: Options = {}
) {
  return new Promise<any>((resolve, reject) => {
    fn()
      .then(resolve)
      .catch((error) => {
        setTimeout(() => {
          if (retries === 1) {
            reject(error)
            return
          }

          // Passing on "reject" is the important part
          retry(fn, {
            retries: retries - 1,
            interval: exponentialBackoff ? interval * 2 : interval,
          }).then(resolve, reject)
        }, interval)
      })
  })
}

Example

const Example = loadable(() => import('./Example'),  {retries: 3})

Pitch

I'm wondering if this is something that could be directly implemented as part of loadable.

enhancement ✨ feature request 🙏

Most helpful comment

Good point. As a matter of fact - our own Sentry is filled with the same reports.

All 9 comments

Good point. As a matter of fact - our own Sentry is filled with the same reports.

If you agree, I'd be happy to work a PR.

That could be a little complicated as right now errors are just thrown, expected to be handled on the nearest ErrorBoundary.
The question - should network problems be still reported (so the error has to be thrown), or in case they are retry-able loadable can swallow them?

I don't know if you are able to differentiate network errors from other kinds of errors when using dynamic imports. Do you know?

Regarding errors just being thrown at the moment, if the retry is optional, we could still retry all errors as it would not be a breaking change. What do you think?

I don't know if you are able to differentiate network errors

You can, but that is abstraction leaking, so you should not try to differentiate.

Regarding errors just being thrown at the moment...

This moment can alter the implementation a lot. Providing ErrorBoundary with built-in retry mechanism and built-in reporting can be a better solution, and actually can be done right now. Well, almost - such error boundary cannot distinguish LoadableError from any error coming from within loaded content. So we are circling back to the problem of "how to differentiate"

I might be missing something but I think the retry function I shared in my issue is only retrying errors related to the dynamic import, not errors within the loaded component.

By the way, I deployed it on our main app in production and almost all the ChunkLoadError are gone.

If only error boundary could detect chunk loading error (thrown by loadable) - then it can do retry.

Wrapping import with any extra code is breaking SSR. There is an update to a babel plugin to make that impossible as well.

Wrapping import with any extra code is breaking SSR. There is an update to a babel plugin to make that impossible as well.

What do you mean by "breaking SSR"? We use this technique on an SSR application and I didn't notice any issues. 🤔

Hello!
Facing the same issue from some time. I would like to try @ValentinH solution, will let you know when it will be tested it on production in my project

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ksntcrq picture ksntcrq  ·  7Comments

ghost picture ghost  ·  4Comments

hemmxwxsoo picture hemmxwxsoo  ·  4Comments

gregberge picture gregberge  ·  6Comments

gregberge picture gregberge  ·  7Comments