ionic-team / capacitor

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

[Feature]: Create some mechanism to notify plugins when the webview crashes #7713

Open laine-hallot opened 1 week ago

laine-hallot commented 1 week ago

Description

It would be nice to have a way for a plugin to tell Capacitor to run some code when the webview crashes

Platforms

Request or proposed solution

Before getting into things I just want to say I'm not sure whether i should create this as an issue or as a discussion so if the latter is more appropriate let me know and I'll post it there instead. Anyways

This is a subject that's been brought up a few times already but the discussions surrounding it never lead to any actual changes in Capacitor's code

Our team wants to use it for logging and looking at prior issues/discussions that use case seems like a pretty common desire (especially on iOS).

I was planning on writing the code for this and making a PR but wanted to make this issue to check if the maintainers are actually interested in adding this feature and to get some input on what the implementation should look like.

Alternatives

For stuff like logging crashes you could always just create a way to opt out of the auto-restart behavior on iOS and let the whole app crash which should let you see a log for that in the App analytics but it seems like Apples policies might be a concern there. https://github.com/ionic-team/capacitor/issues/2379#issuecomment-677496822

Also I think there is a case for not wanting to let plugin developers hook into the something that runs on app crash since putting too much code there could slow things down and lead to crashes being even more annoying to end users.

Additional Information

Android solution

It seems like there's already sort of a way to run code when the webview crashes on android, by having your plugin add a listener for onRenderProcessGone to the bridge with addWebViewListener(). https://github.com/ionic-team/capacitor/blob/09d99ba22677c28bd651f2d3a8f39f33565e0bc0/android/capacitor/src/main/java/com/getcapacitor/BridgeWebViewClient.java#L97-L110 https://github.com/ionic-team/capacitor/blob/09d99ba22677c28bd651f2d3a8f39f33565e0bc0/android/capacitor/src/main/java/com/getcapacitor/Bridge.java#L1520-L1524 https://github.com/ionic-team/capacitor/blob/09d99ba22677c28bd651f2d3a8f39f33565e0bc0/android/capacitor/src/main/java/com/getcapacitor/WebViewListener.java#L9

I while I haven't tried that particular route to get code to run on webview crash it seems like all the pieces are already there for it on android. Between that and the lack of an analogous webview.reload() call when the webview crashes on Android I think the bulk of the discussion here should be focused on iOS.

iOS

I haven't tried all of these but it seems like there's a couple ways we could get plugin code run when the webview restarts (both of them involve calling code from webViewWebContentProcessDidTerminate())

1. Adding to the CAPPlugin interface

This approach involves extending the CAPPlugin interface to include a new method that webViewWebContentProcessDidTerminate() can call, similar to shouldOverrideLoad(). https://github.com/ionic-team/capacitor/blob/e765c0fdb4e8557eda9012cbf89364441d4c84c0/ios/Capacitor/Capacitor/CAPPlugin.h#L34-L41 https://github.com/ionic-team/capacitor/blob/e765c0fdb4e8557eda9012cbf89364441d4c84c0/ios/Capacitor/Capacitor/WebViewDelegationHandler.swift#L79-L95

This is the only method I've actually tried so far and while it seems like it could maybe work the fact that it involves messing with the defenition of the CAPPlugin interface makes me want to avoid it to not risk screwing something up there. I also can't seem to figure our how to override the Objective-C version of the method from the Swift plugin code but I think that's just me being bad at Swift.

2. The NotificationCenter and Observer apis

This route involves defining a new notification name in CAPNotifications https://github.com/ionic-team/capacitor/blob/e765c0fdb4e8557eda9012cbf89364441d4c84c0/ios/Capacitor/Capacitor/CAPNotifications.swift#L8-L19 and having plugins create an Observer for it so that their code gets run when webViewWebContentProcessDidTerminate() triggers the notification.

The main issue I think this approach has is that it seems like there's no guarantee that all Observers will get run before webView.reload() gets called. While that would be annoying in the case where you want to be sure your JS side code knows about the crash as soon as it starts I could also see it being desirable if there's a concerns over the webview restart getting delayed due to slow plugin code.

TL:DR

I want to make a PR to add this feature but need a some questions answered before I can do that:

  1. Do the maintainers even want this feature?
  2. Should this be a method that's part of the CAPPlugin interface or should it be an some kind of event that plugins can listen for?
  3. Should webView.reload() only get called once all plugin handlers for this complete?
peitschie commented 1 week ago

Just for some more background that I'm aware of...

I believe this is already supported on the Android side as part of https://github.com/ionic-team/capacitor/pull/6946

See for tips on how to add a custom WebViewListener to subscribe: https://github.com/ionic-team/capacitor/issues/2645

There's an open request to add a similar mechanism for iOS here: https://github.com/ionic-team/capacitor/issues/7260 which is perhaps all that is needed to support your use-case here?