search

OneSignal / OneSignal-Website-SDK

OneSignal is a push notification service for web and mobile apps. This SDK makes it easy to integrate your website with OneSignal Push Notifications. https://onesignal.com
Other
389 stars 115 forks source link

[Feedback]: "Web Push Troubleshooting" documentation appears out-of-date #992

Closed onethirtyfive closed 1 month ago

onethirtyfive commented 1 year ago

What's on your mind?

My friend, a OneSignal infrastructure eng, was kind enough to refer me for a product engineering role this past week. I spent the evening working on a conversation piece demo I hope to have ready if and when. (Yes, I'm shameless. :))

At one point, I had made an easily-overlooked TS misconfiguration that broke my integration. While fixing, I ran into "Debugging not receiving chrome notifications" in your "Web Push Troubleshooting" doc.

Following step 4 of this section, I opened chrome://serviceworker-internals, clicking "Inspect" on my app's service worker. In the resulting popout devtools console, when I started to enterOneSignal.log.setLevel('trace'), I saw:

image

Curious, I cloned the repo and went digging. Looks like a parallel logging implementation solely for ServiceWorker was introduced by https://github.com/OneSignal/OneSignal-Website-SDK/commit/2550ae5eb4d9818faefdb8e824bdee5459b86cf3.

After familiarizing myself with SDK internals, I tried this in my app's console and it worked:

OneSignal.context.workerMessenger.directPostMessageToSW('os.set_sw_logging', { shouldLog: true })

That section remains useful, imho: great exposure to Chrome's service worker tooling. Thanks for writing it.

rgomezp commented 1 year ago

Thanks for the issue.

So to confirm, you're saying that the normal setLogLevel function is not also setting it on the SW side?

onethirtyfive commented 1 year ago

Right!

Two scenarios for you. Before each, I cleared site data and stopped/unregistered service worker.

Scenario 1: only OneSignal.log.setLevel('trace'):

image

Scenario 2, with a stunning "mask" effect courtesy of Preview.app:

  1. OneSignal.log.setLevel('trace'), then
  2. OneSignal.context.workerMessenger.directPostMessageToSW('os.set_sw_logging', { shouldLog: true }):

image

Here's the code origin of one of the log messages in Scenario 2 which must originate from the service worker:

image

~I wanted to PR a fix for this, but I wasn't too happy with what I came up with.~ It takes a lot of time to learn a mature, feature-rich product, so I took went for two birds, one stone ... took it apart and reassembled it, as a code sample. It's yours if you like it. I'll submit it through hiring channels (email) later today.

In an case, I hope this documents the issue, for now. Appreciate it!

edit: PR attached.