Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add debug, error logs for push notifications and update the service-worker's scope #1119

Merged
merged 9 commits into from
Apr 18, 2024

Conversation

charleslavon
Copy link
Contributor

@charleslavon charleslavon commented Apr 17, 2024

This PR

  • Adds debug and console error logs for improved debugging
  • moves the push-notification worker into a new folder such that the default scope can be used, because there is frequent confusion surrounding the meaning and use of scope. Since a service worker can't have a scope broader than its own location, only use the scope option when you need a scope that is narrower than the default. (reference)

Areas for additional improvement

  1. decouple the registering/de-registering of the push notification service worker from the UX flow of enabling/disabling push notifications. Instead, this worker can be added as an import script into the pwa service worker's config. The benefits would be easier local debugging of changes to push-notifications.js and would ensure that a user's browser would always refresh to the latest version of push-notifications.js without requiring them to disable and re-enable push notifications.
  2. Push Notifications still are often not received by a client, even when notification data is properly traced through the backend flow and messages appear to be sent to the Apple/Google subscription endpoints. Right now we are essentially throwing into the ocean a message-in-a-bottle and hoping that it is received by the 3rd party subscription endpoint handler and then delivered to the user's client. We can find ways to improve this, perhaps by implementing support for read/delivery receipts or error logging.

Copy link

vercel bot commented Apr 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dev-near-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2024 3:32pm
near-discovery ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2024 3:32pm
near-discovery-testnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2024 3:32pm

@charleslavon charleslavon changed the title Set the push notification worker to have a default scope of it's folder path Add debug, error logs for push notifications and update the servier-worker's scope Apr 18, 2024
@charleslavon charleslavon changed the title Add debug, error logs for push notifications and update the servier-worker's scope Add debug, error logs for push notifications and update the service-worker's scope Apr 18, 2024
@charleslavon charleslavon marked this pull request as ready for review April 18, 2024 15:32
@charleslavon charleslavon marked this pull request as draft April 18, 2024 15:32
@charleslavon charleslavon marked this pull request as ready for review April 18, 2024 15:39
@charleslavon charleslavon merged commit 77a1cf5 into develop Apr 18, 2024
7 checks passed
@charleslavon charleslavon deleted the default_scope branch April 18, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release Use when a PR's description is should not be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants