Allow error checking before handling in the overlay

[mohitkyadav]

Motivation

I'm maintaining an old react app, which has a lot of custom exceptions. We recently started using react-refresh-webpack-plugin, and it catches all these exceptions while developing and covers the whole screen. As of now, we have disabled the overlay. I also think developers should have some degree of control over runtime errors.

Proposed Changes

I propose to add an optional custom function in handleRuntimeError which reports the error and its response decides whether this should trigger an overlay.

Somewhat like this:

-function handleRuntimeError(error) {
+function handleRuntimeError(error, options) {
+ const { customChecker } = options
+ if (customChecker && !customChecker(error)) {
+   return
+ }

  if (error && !isWebpackCompileError(error) && currentRuntimeErrors.indexOf(error) === -1) {
    currentRuntimeErrors = currentRuntimeErrors.concat(error);
  }
  debouncedShowRuntimeErrors(currentRuntimeErrors);
}

---

Please also enlighten me if there's already a way to achieve this. I also went through the whole discussion on https://github.com/facebook/create-react-app/issues/6530.

I'd love to open a PR if this issue and proposed solution is legit.

[pmmmwh]

Hi, I'm planning to add something to the overlay that you can import and mark an error as ignored when you catch the error. Does that cover your use case?

I do not want to change the API surface of the overlay, nor do I feel like injecting another user-defined function is ergonomic ...

[mohitkyadav]

Hi, I'm planning to add something to the overlay that you can import and mark an error as ignored when you catch the error. Does that cover your use case?

If I understand correctly I'll have to mark that error in every single place it needs to be ignored?

If yes then it doesn't cover my case.

react-refresh-webpack-plugin is only used in start script of the app if your solution results in importing it inside src/ then I think it's even less ergonomic.

Please correct me if I'm wrong and a usage snippet of the feature you are planning would be helpful for me to understand the approach.

[pmmmwh]

How do you think your proposal can be implemented? I feel quite strongly that most uncaught runtime errors should be warned, can you explain the situation a bit more?

[mohitkyadav]

The Situation:

I have a number of custom Exceptions which are uncaught in a lot of places. When I added react-refresh-webpack-plugin it cause problems for the developers cause they were suddenly seeing this overlay catching these exceptions. For the time being I've disabled the overlay.

I feel quite strongly that most uncaught runtime errors should be warned.

I agree completely. But atm it's not so simple to modify the modules or catching these exceptions.

Plugin is only used in webpack.config.dev.js. Solution I'm proposing is to add an option in

interface ErrorOverlayOptions {
  entry?: string | false;
  module?: string | false;
  sockIntegration?: 'wds' | 'whm' | 'wps' | false | string;
  sockHost?: string;
  sockPath?: string;
  sockPort?: number;
  sockProtocol?: 'http' | 'https' | 'ws' | 'wss';
  useURLPolyfill?: boolean;
+ errorChecker?: function;
}

I'm not fixated on this solution, but I think this can prevent users from needing to write this development related code to ignore errors along with their production business logic.

[pmmmwh]

This doesn't really work though, because errorChecker would be a function in Node.js, and handleRuntimeError runs in the browser.

The API would at least be like so:

interface ErrorOverlayOptions {
  entry?: string | false;
  module?: string | false;
  sockIntegration?: 'wds' | 'whm' | 'wps' | false | string;
  sockHost?: string;
  sockPath?: string;
  sockPort?: number;
  sockProtocol?: 'http' | 'https' | 'ws' | 'wss';
  useURLPolyfill?: boolean;

  // Path to a module containing an export of some function
  errorFilter?: string;
}

This widens the API surface a lot and I'm not sure if the use case really warrant such a big effort, especially when it could make it easy for bad behaviour (e.g. just ignoring everything), effectively making such an overlay useless. We kind of want to intentionally make this difficult enough if we allow it. Also, this module will run in the browser unlike all the other "webpack configuration" code which will still run in Node.js, creating another source of confusion.

There were also previous suggestions: https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/262#issuecomment-736011010