Skip to content

Fix: Push notifications error on unsupported browsers#868

Closed
sentry[bot] wants to merge 1 commit into
devfrom
seer/fix-push-notifications-unsupported
Closed

Fix: Push notifications error on unsupported browsers#868
sentry[bot] wants to merge 1 commit into
devfrom
seer/fix-push-notifications-unsupported

Conversation

@sentry
Copy link
Copy Markdown

@sentry sentry Bot commented May 19, 2026

Description

This PR addresses the unhandled error "Push messaging is not supported in this browser." (SABLE-TQ) that occurred when enablePushNotifications() was called on browsers lacking serviceWorker or PushManager APIs.

Root Cause:

  1. The togglePusher() function, invoked on every visibilitychange event, called enablePushNotifications() without first checking for browser push notification capability.
  2. enablePushNotifications() would then throw an unhandled error if the necessary browser APIs (serviceWorker, PushManager) were absent.
  3. The usePushNotifications setting could be enabled (and defaulted to true on mobile) even on browsers that do not support push notifications (e.g., DuckDuckGo, Safari iOS, Firefox iOS), leading to repeated errors.

Changes Made:

  1. Introduced isPushSupported() utility: A new helper function isPushSupported() was added to PushNotifications.tsx to check for the presence of serviceWorker and PushManager APIs.
  2. Guarded togglePusher(): togglePusher() now performs an early return if !isPushSupported(), preventing calls to enablePushNotifications() on unsupported browsers and eliminating the repeated errors on visibility changes.
  3. Graceful enablePushNotifications(): The enablePushNotifications() function now returns gracefully instead of throwing an error when push is unsupported, providing a defense-in-depth mechanism.
  4. Updated default setting value: The default value for usePushNotifications in settings.ts now includes a check for isPushSupported(), ensuring that the setting is not enabled by default on unsupported browsers.
  5. Hid settings UI: The "Background Push Notifications" setting tile in SystemNotification.tsx is now hidden if the browser does not support push notifications, improving the user experience by not presenting unavailable features.

Fixes SABLE-TQ

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

AI disclosure:

  • Partially AI assisted (clarify which code was AI assisted and briefly explain what it does).
  • Fully AI generated (explain what all the generated code does in moderate detail).

Fixes SABLE-TQ

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Missing changeset

This pull request does not include a changeset. Please add one before requesting review so the change is properly documented and included in the release notes.

How to add a changeset:

  1. Run pnpm run document-change (interactive) and commit the generated file, or
  2. Manually create .changeset/<descriptive-name>.md:
---
default: patch
---

Short user-facing summary of the change.

Replace patch with major, minor, patch, docs, or note as appropriate.

📖 Read more in CONTRIBUTING.md.

If this PR is internal/maintenance with no user-facing impact, a maintainer can add the internal label to skip this check.

@github-actions
Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

Status Preview URL Commit Alias Updated (UTC)
✅ Deployment successful! https://pr-868-sable.raspy-dream-bb1d.workers.dev e9dbda2 pr-868 Tue, 19 May 2026 10:47:06 GMT

@dozro
Copy link
Copy Markdown
Member

dozro commented May 20, 2026

PR by sentry? huh?

@7w1
Copy link
Copy Markdown
Member

7w1 commented May 20, 2026

bad bot at the very least pr opener needs to be human

@7w1 7w1 closed this May 20, 2026
@dozro
Copy link
Copy Markdown
Member

dozro commented May 20, 2026

bad bot at the very least pr opener needs to be human

Yeah would be preferable lol

@7w1 7w1 deleted the seer/fix-push-notifications-unsupported branch May 20, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants