Closed Bug 1250905 Opened 8 years ago Closed 8 years ago

Can't debug service worker from https://mdn.github.io/sw-test/

Categories

(DevTools :: about:debugging, defect)

47 Branch
defect
Not set
normal

Tracking

(firefox47 affected)

RESOLVED DUPLICATE of bug 1153292
Tracking Status
firefox47 --- affected

People

(Reporter: janx, Assigned: jdescottes)

References

Details

STR:
1. Open tab on about:debugging#workers
2. Open tab on https://mdn.github.io/sw-test/
3. In about:debugging, click on "Debug" next to https://mdn.github.io/sw-test/sw.js

Expected:
- A toolbox opens, letting you debug this worker.

Actual result:
- No toolbox opens.

From the Browser Console:

Service worker installing app.js:7:7

Service worker event waitUntil() was passed a promise that rejected with 'TypeError: Cache got basic response with bad status 404 while trying to add request https://mdn.github.io/sw-test/gallery/'. sw.js:2:2

error occurred while processing 'attach: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIServiceWorkerManager.getRegistrationByPrincipal]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/worker.js :: WorkerActor.prototype._getServiceWorkerInfo :: line 133"  data: no]
Stack: WorkerActor.prototype._getServiceWorkerInfo@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/worker.js:133:16
WorkerActor.prototype.onAttach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/worker.js:66:22
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1643:15
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:569:11
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
Line: 133, column: 0
 main.js:1512:0

"onPacket threw an exception: Error: Server did not specify an actor, dropping packet: {"error":"unknownError","message":"error occurred while processing 'attach: [Exception... \"Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIServiceWorkerManager.getRegistrationByPrincipal]\"  nsresult: \"0x80004005 (NS_ERROR_FAILURE)\"  location: \"JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/worker.js :: WorkerActor.prototype._getServiceWorkerInfo :: line 133\"  data: no]\nStack: WorkerActor.prototype._getServiceWorkerInfo@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/worker.js:133:16\nWorkerActor.prototype.onAttach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/worker.js:66:22\nDSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1643:15\nLocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:569:11\nexports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14\nexports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14\nLine: 133, column: 0"}
Stack: DebuggerClient.prototype.onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:948:9
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:569:11
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
Line: 948, column: 9"
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
In the install event callback [1], the cache is filled with several paths, including `/sw-test/gallery/` which actually points to a 404. addCache then throws an exception.
On Firefox, this seems to force the serviceworker to unregister : https://mdn.github.io/sw-test/sdfsdfsdf should display a Darth Vader picture ; it does on Chrome, but does not on Firefox.

Removing this path from the array allows the worker to be installed without any error, and to be debugged.
(can be tested at https://juliandescottes.github.io/sw-test/ )

Every time I tested, the worker from https://mdn.github.io appears in about:debugging but disappears after ~5 seconds.

From this observation, I suppose the following happens:
- serviceworker is registered
- `workerListChanged` event is fired
- workers-tab is updated
- serviceworker's "install" event handler is called and throws an exception
- serviceworkers is removed
- `workerListChanged` event is fired
- workers-tab is updated

According to the service workers specs [2], Firefox's behavior is correct.

On our side we should only list service workers in the "Activated" state.
Moreover, it would be nice to also display the workers that failed to install, so that the user knows they can not be debugged and why.

[1] https://github.com/mdn/sw-test/blob/gh-pages/sw.js
[2] https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#installation-algorithm
(In reply to Julian Descottes [:jdescottes] from comment #1)
> 
> On our side we should only list service workers in the "Activated" state.
> Moreover, it would be nice to also display the workers that failed to
> install, so that the user knows they can not be debugged and why.

janx: I think as a first step, just making sure we only display workers that successfully installed is the best. Any comment/objection before I try to implement this here?
Flags: needinfo?(janx)
Thanks a lot for investigating this problem, and for checking with the specs!

(In reply to Julian Descottes [:jdescottes] from comment #1)
> On Firefox, this seems to force the serviceworker to unregister :
> https://mdn.github.io/sw-test/sdfsdfsdf should display a Darth Vader picture
> ; it does on Chrome, but does not on Firefox.

So Chrome doesn't clear the Service Worker registration even when there was an uncaught error during install, which indeed seems contrary to the specs.

It even allows the resulting Service Worker to be inspected with their Devtools (from chrome://serviceworkers-internals), which opens a Toolbox with a Console where you can see the 404 error:

"Failed to load resource: the server responded with a status of 404 (Not Found) https://mdn.github.io/sw-test/gallery/".

Then again the specs say we should delete the registration record, so continuing to show it with an error state also seems contrary to the specs.

I'm wondering which behavior would be the most useful / less confusing for users. Chrome's behavior seems useful even if its "Installation Status: ACTIVATED" is lying somewhat.

> Removing this path from the array allows the worker to be installed without
> any error, and to be debugged.
> (can be tested at https://juliandescottes.github.io/sw-test/ )

Thanks for confirming this!

> From this observation, I suppose the following happens:
> - serviceworker is registered
> - `workerListChanged` event is fired
> - workers-tab is updated
> - serviceworker's "install" event handler is called and throws an exception
> - serviceworker is removed
> - `workerListChanged` event is fired
> - workers-tab is updated

Looks correct.

> On our side we should only list service workers in the "Activated" state.
> Moreover, it would be nice to also display the workers that failed to
> install, so that the user knows they can not be debugged and why.

Actually, I think we could even go a step beyond and address bug 1153292, by showing the installation status of each Service Worker (i.e. "installing", "waiting", "active", and maybe we could go a little off-spec by using a 4th install status like "failed" or "error").

Helen had some cool UX ideas for this in bug 1220747, i.e. using a text label next to the SW.
Depends on: 1153292
Flags: needinfo?(janx)
Should we close this bug as duplicate of Bug 1153292 ?
Flags: needinfo?(janx)
(In reply to Julian Descottes [:jdescottes] from comment #4)
> Should we close this bug as duplicate of Bug 1153292 ?

Good idea, let's do that.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: needinfo?(janx)
Resolution: --- → DUPLICATE
Opened a PR on Github [1] to fix the issue described here.

[1] https://github.com/mdn/sw-test/pull/12
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.