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

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

Closed
onethirtyfive opened this issue Feb 18, 2023 · 2 comments
Closed

Comments

@onethirtyfive
Copy link

onethirtyfive commented Feb 18, 2023

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 2550ae5.

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
Copy link
Contributor

rgomezp commented Feb 22, 2023

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
Copy link
Author

onethirtyfive commented Mar 1, 2023

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.

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

No branches or pull requests

3 participants