ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚡️
https://capacitorjs.com
MIT License
12.03k stars 999 forks source link

feat: Add additional context on plugin errors #6279

Open Dennitz opened 1 year ago

Dennitz commented 1 year ago

Feature Request

Description

Currently, the information included in plugin errors doesn't always make it obvious where an error gets triggered, nor which plugin and plugin method is involved. It would be nice if additional information could be added. I'd be happy to provide a PR for this, upon some guidance.

Specifically this applies for UNIMPLEMENTED errors.

For example, we we've been getting the error Error: not implemented. The full stack trace looks like this:

207875386-24b7669f-22bf-4937-8491-0ad28c5a3eb3

Ultimately, this error got triggered by calling LocalNotifications.listChannels() of the LocalNotifications plugin on iOS. The error is expected as this method isn't implemented on iOS, but we didn't realise this at first.

The problem is that there is no way to know what is causing Error: not implemented just by looking at the error and its stack trace.

The information provided by the error doesn't reveal:

This makes it rather hard to find where certain errors are originating from.

Platform(s)

Preferred Solution

There are multiple solutions I can think of:

Include the location of the calling code in the stack trace

This would be the ideal solution.

If the line that is calling the problematic plugin method, e.g. the line where LocalNotifications.listChannels() gets called, was included in the stack trace, then it would be obvious where an error is originating from.

I'm not sure if this can be achieved though.

Update the default error message

Instead of having a default error message of Error: not implemented, the default message could include the name of the plugin, the name of the plugin method, and optionally the platform.

So for example, the message could then be Error: LocalNotifications.listChannels is not implemented on iOS.

This isn't a great solution though, as it would require a breaking change. And it also wouldn't work if a plugin method provides a custom error message, like here for example.

Include plugin and method names as additional properties on the error object

This seems like a straightforward solution to implement. It wouldn't be quite as user-friendly as the other approaches though, as some work is required by the developer that is using Capacitor to make this useful.

The approach here would be to add pluginId and methodName to the error object. This requires a small change in native-bridge.js, here for iOS, and here for Android. Essentially:

// looks like we've got a stored call
if (result.error) {
    // ensure stacktraces by copying error properties to an Error
-    result.error = Object.keys(result.error).reduce((err, key) => {
+    const newError = Object.keys(result.error).reduce((err, key) => {
        // use any type to avoid importing util and compiling most of .ts files
        err[key] = result.error[key];
        return err;
    }, new cap.Exception(''));
+ 
+   newError["pluginId"] = result.pluginId
+   newError["methodName"] = result.methodName
+ 
+   result.error = newError
}

As a user of Capacitor, one could then have a global error handler along the lines of:

      window.addEventListener('unhandledrejection', err => {
          if (err.reason?.code === 'UNIMPLEMENTED'
              && err.reason?.pluginId
              && err.reason?.methodName
          ) {
              // Get platform: 'android', 'ios', or 'web'.
              // Can be done for example using the `@capacitor/device` plugin.
              const platform = 'xxx'

              // Do something else with the error here. E.g. send it to Sentry, etc.
              console.log(`${err.reason.pluginId}.${err.reason.methodName} is not implemented on platform ${platform}.`)
          }
      })

And this would print for example:

LocalNotifications.listChannels is not implemented on platform xxx.

Not as good as having the line where the problematic method is called mentioned in the stack trace, but this would make it easy enough to find were an error gets triggered.

Alternatives

I haven't found any solution that doesn't involve making changes to internal Capacitor code.

Additional Context

Minimal sample application: https://github.com/Dennitz/capacitor-error-reporting-example

The application includes examples of UNIMPLEMENTED errors for both iOS and Android.

The example for iOS calls LocalNotifications.listChannels, which isn't implemented on iOS.

And the Android example calls Camera.getLimitedLibraryPhotos, which isn't implemented on Android.

When testing the iOS example, please comment out the Android example, i.e. make sure these lines are commented out.

Vice versa, when testing the Android example, make sure these lines are commented out.

In both examples, a plugin method is called that isn't implemented on the platform. The error is then caught, and logged to the console.

There it can be seen that the error and its stack trace don't make it visible what plugin and plugin method are causing the error.

Example from iOS:

Screenshot 2023-02-07 at 12 37 54

With the top item of the stack trace linking here (which doesn't reveal anything either):

Screenshot 2023-02-07 at 13 28 03

Example from Android:

Screenshot 2023-02-07 at 12 37 04
anonimitoraf commented 1 year ago

Any updates on this? We're also experiencing the same problem.

Did you manage to find any workarounds @Dennitz?