wix-incubator / react-module-container

Small library for building micro apps in React and Angular
MIT License
69 stars 18 forks source link

When `resolve` function fails there is no way to know about it #70

Closed sidoruk-sv closed 4 years ago

sidoruk-sv commented 4 years ago

For now nothing rendered and there is no errors for such case:

class MyReactCompRejected extends ReactLazyComponent {
  constructor(props) {
    super(props, {
      resolve: async () => {
        throw new Error('some error in resolve');
      },
    });
  }
}

related code: https://github.com/wix/react-module-container/blob/master/src/base-lazy-component.js#L22 Mention that calling of resolve() is not wrapped with try catch, so componentWillMount fails in silence.

ronenst commented 4 years ago

Actually, we do emit an event, BM catches this event and reports to Sentry, but we would like to add onSuccess/onLoad and onError props (instead since the events are unfocused). https://github.com/wix/react-module-container/blob/master/src/base-lazy-component.js#L27

sidoruk-sv commented 4 years ago

Actually, we do emit an event, BM catches this event and reports to Sentry

How do you catch that? cause our listener for reactModuleContainer.error didn't helped, and we spent some time to understand that problem was thrown from resolve method

sidoruk-sv commented 4 years ago

Am I right that due to that fact that resolved was called before Promise.all any errors from it didn't get to Promise.all().catch and there is a need to add additional try catch around line 22

sidoruk-sv commented 4 years ago

Hmmm.. seems like I'm not correct, so will digg for another reason why our listener for reactModuleContainer.error was not triggered and we could not identify the problem.

sidoruk-sv commented 4 years ago

Found my problem: we add reactModuleContainer.error listener after the error occurred(

So we have no chances to get any notifications from that)

ronenst commented 4 years ago

It is true though btw that if resolve throws synchronously then the error won't be caught but in that case I believe a React ErrorBoundary can help.