Skip to content

Commit 2c92ecc

Browse files
fix(snaps): Improve WebView loading detection (#27626)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Apparently `onLoadEnd` may trigger even if loading the WebView fails. Therefore, we delay resolving the WebView promise for a Snap until the first message is received from the WebView. We strictly use `onLoadEnd` to detect errors instead. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** https://consensyssoftware.atlassian.net/browse/WPC-517 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the readiness/creation flow for snaps execution WebViews to resolve only after the first `onMessage`, and uses `onLoadEnd` solely for error rejection; this could affect snap startup timing or lead to hangs if a WebView never posts its initial message. > > **Overview** > Adjusts `SnapsExecutionWebView` so `createWebView()` no longer resolves when `onLoadEnd` fires; it now resolves on the **first `onMessage` received** from the WebView. > > `onLoadEnd` is repurposed to **only detect load failures**, rejecting the promise when the event contains an error `code`, and the explicit `onError` handler is removed along with updated WebView event typings. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c54c970. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 290655d commit 2c92ecc

1 file changed

Lines changed: 44 additions & 36 deletions

File tree

app/lib/snaps/SnapsExecutionWebView.tsx

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
22
import React, { Component } from 'react';
3-
import { View, NativeSyntheticEvent } from 'react-native';
3+
import { View } from 'react-native';
44
import { WebViewMessageEvent, WebView } from '@metamask/react-native-webview';
55
import { createStyles } from './styles';
66
import { WebViewInterface } from '@metamask/snaps-controllers/react-native';
7-
import { WebViewError } from '@metamask/react-native-webview/src/WebViewTypes';
7+
import {
8+
WebViewNavigationEvent,
9+
WebViewErrorEvent,
10+
} from '@metamask/react-native-webview/src/WebViewTypes';
811
import { PostMessageEvent } from '@metamask/post-message-stream';
912
// @ts-expect-error Types are currently broken for this.
1013
import WebViewHTML from '@metamask/snaps-execution-environments/dist/webpack/webview/index.html';
1114
import { EmptyObject } from '@metamask/snaps-sdk';
12-
import { assert } from '@metamask/utils';
15+
import { assert, hasProperty } from '@metamask/utils';
1316
import Logger from '../../util/Logger';
1417

1518
const styles = createStyles();
@@ -25,8 +28,7 @@ interface WebViewState {
2528
listener?: (event: PostMessageEvent) => void;
2629
props: {
2730
onWebViewMessage: (data: WebViewMessageEvent) => void;
28-
onWebViewLoad: () => void;
29-
onWebViewError: (error: NativeSyntheticEvent<WebViewError>) => void;
31+
onWebViewLoad: (event: WebViewNavigationEvent | WebViewErrorEvent) => void;
3032
ref: (ref: WebView) => void;
3133
};
3234
}
@@ -44,34 +46,46 @@ export class SnapsExecutionWebView extends Component {
4446

4547
createWebView(jobId: string) {
4648
const promise = new Promise<WebViewInterface>((resolve, reject) => {
47-
const onWebViewLoad = () => {
48-
const api = {
49-
injectJavaScript: (js: string) => {
50-
assert(
51-
this.webViews[jobId]?.ref,
52-
'Snaps execution webview reference not found.',
53-
);
54-
this.webViews[jobId].ref?.injectJavaScript(js);
55-
},
56-
registerMessageListener: (
57-
listener: (event: PostMessageEvent) => void,
58-
) => {
59-
if (this.webViews[jobId]) {
60-
this.webViews[jobId].listener = listener;
61-
}
62-
},
63-
unregisterMessageListener: (
64-
_listener: (event: PostMessageEvent) => void,
65-
) => {
66-
if (this.webViews[jobId]) {
67-
this.webViews[jobId].listener = undefined;
68-
}
69-
},
70-
};
71-
resolve(api);
49+
const api = {
50+
injectJavaScript: (js: string) => {
51+
assert(
52+
this.webViews[jobId]?.ref,
53+
'Snaps execution webview reference not found.',
54+
);
55+
this.webViews[jobId].ref?.injectJavaScript(js);
56+
},
57+
registerMessageListener: (
58+
listener: (event: PostMessageEvent) => void,
59+
) => {
60+
if (this.webViews[jobId]) {
61+
this.webViews[jobId].listener = listener;
62+
}
63+
},
64+
unregisterMessageListener: (
65+
_listener: (event: PostMessageEvent) => void,
66+
) => {
67+
if (this.webViews[jobId]) {
68+
this.webViews[jobId].listener = undefined;
69+
}
70+
},
71+
};
72+
73+
const onWebViewLoad = (
74+
event: WebViewNavigationEvent | WebViewErrorEvent,
75+
) => {
76+
if (hasProperty(event.nativeEvent, 'code')) {
77+
reject(
78+
new Error(
79+
`Snaps execution webview failed to load with error code: ${event.nativeEvent.code}`,
80+
),
81+
);
82+
}
7283
};
7384

7485
const onWebViewMessage = (data: WebViewMessageEvent) => {
86+
// We resolve the promise on the first message received
87+
resolve(api);
88+
7589
if (this.webViews[jobId]?.listener) {
7690
try {
7791
this.webViews[jobId].listener?.(
@@ -83,10 +97,6 @@ export class SnapsExecutionWebView extends Component {
8397
}
8498
};
8599

86-
const onWebViewError = (error: NativeSyntheticEvent<WebViewError>) => {
87-
reject(error);
88-
};
89-
90100
const setWebViewRef = (ref: WebView) => {
91101
if (this.webViews[jobId]) {
92102
this.webViews[jobId].ref = ref;
@@ -96,7 +106,6 @@ export class SnapsExecutionWebView extends Component {
96106
this.webViews[jobId] = {
97107
props: {
98108
onWebViewLoad,
99-
onWebViewError,
100109
onWebViewMessage,
101110
ref: setWebViewRef,
102111
},
@@ -126,7 +135,6 @@ export class SnapsExecutionWebView extends Component {
126135
ref={props.ref}
127136
source={{ html: WebViewHTML, baseUrl: 'https://localhost' }}
128137
onMessage={props.onWebViewMessage}
129-
onError={props.onWebViewError}
130138
onLoadEnd={props.onWebViewLoad}
131139
originWhitelist={['*']}
132140
javaScriptEnabled

0 commit comments

Comments
 (0)