[$250] [Sentry: APP-2D7] Web Notification illegal constructor crash

[mountiny]

Sentry

https://expensify.sentry.io/issues/APP-2D7

Impact (snapshot at filing)

• Users (total since first seen): 389
• Events: 1,535
• Users (last 14d, 9.4.x releases): 53
• First seen: 2025-12-09
• Last seen: ongoing
• Platform: web (notably Samsung Internet on Android)
• App version(s): 9.4.5–9.4.9
• Affected route(s): /r/:reportID
• Mechanism: auto.browser.global_handlers.onunhandledrejection

Stack trace (top frames, first-party only)

src/libs/Notification/LocalNotification/BrowserNotifications.ts:58
new Notification(title, { body, icon, data, ... })

Suspected cause

BrowserNotifications uses new Notification() directly, which is not supported in all browsers (e.g. Samsung Internet); should use ServiceWorkerRegistration.showNotification() instead.

Reproduction

Unknown — see Sentry events linked from the Sentry issue.

Related

• Prior GH issues: none

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022067266019042576521
• Upwork Job ID: 2067266019042576521
• Last Price Increase: 2026-06-17

Issue Owner

Current Issue Owner: @lorretheboy

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022067266019042576521

[godzillaxbox19982014-lgtm]

Proposal

Please re-state the problem that we are trying to solve in this issue.

[$250] [Sentry: APP-2D7] Web Notification illegal constructor crash

What is the root cause of that problem?

I traced this to the path that handles APP, reportID, LocalNotification:

• src/libs/Notification/LocalNotification/BrowserNotifications.ts:17 (BrowserNotifications): const notificationCache: Record<string, Notification> = {};
• src/libs/Notification/LocalNotification/BrowserNotifications.ts:23 (canUseBrowserNotifications): function canUseBrowserNotifications(): Promise<boolean> {
• src/libs/Notification/LocalNotification/BrowserNotifications.ts:24 (getStatus): return NotificationPermission.getStatus().then((status) => {
• The local evidence matches this recurring failure shape: native callback assumes current policy/report still exists. Here, src/libs/Notification/LocalNotification/BrowserNotifications.ts:17 and src/libs/Notification/LocalNotification/BrowserNotifications.ts:23 do not agree on the same APP, reportID, LocalNotification value, so the user-visible path can show a missing or stale state even though nearby data already has the needed context.

What changes do you think we should make in order to solve the problem?

• Update src/libs/Notification/LocalNotification/BrowserNotifications.ts:17 to derive APP, reportID, LocalNotification from the matched app state path that owns this flow before the value is consumed.
• Carry the same normalized value through src/libs/Notification/LocalNotification/BrowserNotifications.ts:23; that follows the safer family move to add the guard at the ownership boundary, not only at render, so the write/optimistic/read paths agree instead of only changing the visible component.
• Keep the change inside the Sentry Crash, Null/Undefined, Native Hybrid surface and avoid changing do not hide crashes with blanket catch, native platform differences.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

1. Add or update a regression in tests/unit/**/*.ts for the exact issue flow: [$250] [Sentry: APP-2D7] Web Notification illegal constructor crash.
2. Cover the counter-case near src/libs/Notification/LocalNotification/BrowserNotifications.ts:17 so valid existing APP, reportID, LocalNotification behavior still works.
3. Manually repeat the issue steps after the patch and confirm the visible state/action is no longer stale.

What alternative solutions did you explore? (Optional)

• I would not add a broad new condition outside these referenced paths; the safer fix is to keep APP, reportID, LocalNotification consistent at the first read/write boundary shown above.

[lorretheboy]

🚨 Edited by proposal-police: This proposal was edited at 2026-06-17 15:47:29 UTC.

Proposal

What is the root cause of that problem?

push() shows a notification by calling the new Notification() constructor directly:

App/src/libs/Notification/LocalNotification/BrowserNotifications.ts

Lines 58 to 64 in 6ea10e7

	notificationCache[notificationID] = new Notification(title, {
	body,
	icon: SafeString(icon),
	data,
	silent: true,
	tag,
	});

On Android-based browsers (Samsung Internet, Chrome on Android) the page context is forbidden from constructing a Notification; the constructor throws TypeError: Illegal constructor. These browsers only allow notifications via ServiceWorkerRegistration.showNotification().

The guard that runs first only checks permission, never whether the constructor is usable:

App/src/libs/Notification/notificationPermission/index.ts

Lines 13 to 18 in 6ea10e7

	getStatus(): Promise<NotificationPermissionStatus> {
	if (typeof window === 'undefined' || !window.Notification) {
	return Promise.resolve('denied');
	}
	return Promise.resolve(toStatus(Notification.permission));
	},

So the chain is: window.Notification exists and Notification.permission === 'granted' → canUseBrowserNotifications() resolves true → new Notification(...) throws. Because that throw happens inside the .then() callback at line 51 with no .catch(), it surfaces as an unhandled promise rejection → the global onunhandledrejection handler reports it (APP-2D7). This matches every symptom: web-only, /r/:reportID, Samsung Internet on Android.

What changes do you think we should make in order to solve the problem?

There are two distinct things here: (1) stop the crash, and (2) actually deliver the notification on affected browsers. They have very different cost/risk, so I'd land them separately.

Step 1 — stop the unhandled rejection (the actual reported bug). Wrap the construction in try/catch and add a .catch() to the promise chain so an unsupported constructor degrades to "no local notification" instead of an unhandled rejection. This alone resolves APP-2D7 with near-zero regression risk:

App/src/libs/Notification/LocalNotification/BrowserNotifications.ts

Lines 51 to 78 in 6ea10e7

	canUseBrowserNotifications().then((canUseNotifications) => {
	if (!canUseNotifications) {
	return;
	}
	// We cache these notifications so that we can clear them later
	const notificationID = Str.guid();
	notificationCache[notificationID] = new Notification(title, {
	body,
	icon: SafeString(icon),
	data,
	silent: true,
	tag,
	});
	if (!silent) {
	playSound(SOUNDS.RECEIVE);
	}
	notificationCache[notificationID].onclick = () => {
	onClick();
	window.parent.focus();
	window.focus();
	notificationCache[notificationID].close();
	};
	notificationCache[notificationID].onclose = () => {
	delete notificationCache[notificationID];
	};
	});
	}

Step 2 — restore delivery on affected browsers via the service worker. Prefer registration.showNotification(title, {body, icon, data, silent, tag}) when a registration is available, falling back to the (try/catch-guarded) constructor only when there is none. A service worker is already registered in production:

App/src/setup/platformSetup/index.ts

Lines 74 to 82 in 6ea10e7

	if (Config.IS_IN_PRODUCTION && 'serviceWorker' in navigator) {
	window.addEventListener('load', () => {
	navigator.serviceWorker.register('/service-worker.js').catch((error) => {
	// Soft-fail: app must continue working even if SW registration fails (Safari Private mode, etc.).
	// eslint-disable-next-line no-console
	console.warn('[SW] registration failed', error);
	});
	});
	}

Important feasibility constraint that changes the shape of Step 2: notifications shown via showNotification() do not return a Notification instance, so the current click/close wiring and the in-memory notificationCache cannot work as-is:

App/src/libs/Notification/LocalNotification/BrowserNotifications.ts

Lines 68 to 76 in 6ea10e7

	notificationCache[notificationID].onclick = () => {
	onClick();
	window.parent.focus();
	window.focus();
	notificationCache[notificationID].close();
	};
	notificationCache[notificationID].onclose = () => {
	delete notificationCache[notificationID];
	};

Clicks on an SW-shown notification can only be observed inside the service worker via the notificationclick event — the page cannot attach onclick to them, so there is no "equivalent app-level listener." But our service worker is fully generated by Workbox GenerateSW, which emits the entire file and offers no hook to add a notificationclick listener:

App/config/webpack/webpack.common.ts

Lines 135 to 161 in 6ea10e7

	new GenerateSW({
	clientsClaim: true,
	skipWaiting: true,
	// Cap is generous on purpose: the vendor (~6.5 MiB), main (~5.5 MiB),
	// authScreens.prefetch (~6.3 MiB) chunks and canvaskit.wasm (~7.6 MiB) are
	// all critical for offline boot, so we precache the lot. Everything in the
	// App build is content-hashed, so growth here only costs first-install bytes.
	maximumFileSizeToCacheInBytes: 10 * 1024 * 1024,
	// Single-page app: any unmatched navigation should serve the cached app shell.
	navigateFallback: '/index.html',
	// Don't fall back for asset-like or .well-known requests.
	navigateFallbackDenylist: [/^\/_/, /^\/\.well-known/, /\/[^/?]+\.[^/]+$/],
	runtimeCaching: [
	{
	// Same-origin user media (receipts, chat attachments) — cache opportunistically
	// so they're viewable on offline refresh. The function below is serialized into
	// the generated service worker, so it executes in the SW context where
	// `sameOrigin` is the appropriate Workbox match (no need to read `self.location`).
	urlPattern: ({sameOrigin, url, request}) => sameOrigin && (request.destination === 'image' || /\/(receipts|chat-attachments)\//.test(url.pathname)),
	handler: 'StaleWhileRevalidate',
	options: {
	cacheName: 'user-media',
	expiration: {maxEntries: 200, maxAgeSeconds: 7 * 24 * 60 * 60},
	},
	},
	],
	}),

So Step 2 requires migrating the build from GenerateSW to InjectManifest with a small custom SW source that (a) handles notificationclick (focus an existing client or open /r/:reportID, posting a message back to the page so existing onClick behavior is preserved), and (b) lets clearNotifications use registration.getNotifications() + notification.close() (matching on data) instead of the cache:

App/src/libs/Notification/LocalNotification/BrowserNotifications.ts

Lines 184 to 191 in 6ea10e7

	clearNotifications(shouldClearNotification: (notificationData: LocalNotificationData) => boolean) {
	for (const notification of Object.values(notificationCache)) {
	if (!shouldClearNotification(notification.data as LocalNotificationData)) {
	continue;
	}
	notification.close();
	}
	},

Because Step 2 touches the build pipeline and SW lifecycle, I'd ship Step 1 first to kill the crash immediately, then do Step 2 as a scoped follow-up rather than blocking the crash fix on a build migration.

What alternative solutions did you explore? (Optional)

• Use registration.showNotification() + notificationclick without changing the build (the approach in the other posted proposals). This is not implementable on main: with GenerateSW there is nowhere to register a notificationclick handler, and clicks on SW notifications are not observable from the page, so click-to-open-report would silently break. Hence the InjectManifest migration called out above.
• try/catch only, no SW delivery. This is exactly Step 1 and is sufficient to close APP-2D7, but it permanently drops local notifications on Samsung Internet, so I scoped Step 2 to recover them.
• Feature-detect and disable local notifications on affected browsers. Strictly worse than Step 2 — it removes a working feature for those users instead of routing them through the supported SW path.

[MelvinBot]

Proposal

What is the root cause of that problem?

push() shows notifications by calling the new Notification() constructor directly at BrowserNotifications.ts:58.

On Android-based browsers (Samsung Internet, Chrome on Android) the Notification constructor is forbidden and throws TypeError: Illegal constructor — these browsers only permit notifications via ServiceWorkerRegistration.showNotification().

The guard that runs first, canUseBrowserNotifications(), only checks permission, not whether the constructor is usable. It relies on NotificationPermission.getStatus(), which returns granted whenever window.Notification exists and permission is granted.

So the failure chain is:

window.Notification exists + permission granted → canUseBrowserNotifications() resolves true → new Notification(...) throws → because the throw happens inside the .then() callback at line 51 with no .catch(), it surfaces as an unhandled promise rejection → caught by the global onunhandledrejection handler → reported as APP-2D7.

This explains every symptom in the report: web-only, /r/:reportID (report comment notifications), Samsung Internet on Android, and the auto.browser.global_handlers.onunhandledrejection mechanism.

What changes do you think we should make in order to solve the problem?

Prefer the service-worker notification path, which is the only supported way to show notifications on the affected browsers. The app already registers a service worker in production.

In push():

• When a service-worker registration is available, use registration.showNotification(title, {body, icon, data, silent, tag}) instead of new Notification(...).
• Fall back to the direct constructor only when no registration exists, and wrap that construction in try/catch.
• Add a .catch() to the canUseBrowserNotifications() chain so any unsupported path degrades gracefully instead of becoming an unhandled rejection.

Because showNotification() does not return a Notification instance, the click/close wiring and the in-memory notificationCache (lines 17 + 68–76 + 184–191) must move to the SW model: handle clicks via the notificationclick event, and implement clearNotifications with registration.getNotifications() + notification.close() (matching on data) instead of the cache.

What alternative solutions did you explore? (Optional)

Minimal stop-the-bleed: wrap the new Notification(...) call in try/catch and add .catch() to the promise chain. This silences the crash but drops the notification entirely on affected browsers, so the service-worker approach is preferred for actually delivering them.

Note on existing proposals

Two proposals are already posted. lorretheboy's proposal correctly and thoroughly identifies the root cause and a sound fix — this analysis independently confirms it against the current code. The earlier proposal by godzillaxbox19982014-lgtm is generic boilerplate that does not engage with the actual Notification constructor mechanism and should be disregarded.

---

Next Steps for Contributor+ team:
To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/93835#issuecomment-4732180029) to create a draft PR.
To refine: @MelvinBot <your feedback>
To reject: Explain why you are rejecting Melvin's proposal.

[aswin-s]

🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

[github-actions]

⚠️ @aswin-s Your proposal is a duplicate of an already existing proposal and has been automatically withdrawn to prevent spam. Please review the existing proposals before submitting a new one.