Closed
Bug 1233798
Opened 8 years ago
Closed 8 years ago
report to console when service worker register fails due to mime-type issues
Categories
(Core :: DOM: Service Workers, defect)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: bkelly, Assigned: asuth)
References
(Blocks 1 open bug)
Details
(Whiteboard: [tw-dom])
Attachments
(1 file)
8.18 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
I just helped someone diagnose a bug in their node server where they could not register a service worker. It turns out they were not setting the mime-type for the script. This could have been resolved a lot faster if we simply logged a message for this failure.
Reporter | ||
Updated•8 years ago
|
Whiteboard: [tw-dom]
Reporter | ||
Updated•8 years ago
|
Blocks: dt-service-worker
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
Build on bug 1267473 patch adding logging for the missing/bad MIME type case. If I've got any major idiomatic issues, feel free to just comment on that one and I'll propagate through. Error looks like: """ Failed to register/update a ServiceWorker: Non-JS Content-Type of ‘text/plain’ received for script ‘http://mochi.test:8888/tests/dom/workers/test/serviceworkers/sw_bad_mime_type.js’. """
Attachment #8768268 -
Flags: review?(bkelly)
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8768268 [details] [diff] [review] MIME type error message and test v1, layers on top of bug 1267473 fix. Review of attachment 8768268 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: dom/locales/en-US/chrome/dom/dom.properties @@ +210,5 @@ > ServiceWorkerScopePathMismatch=Failed to register a ServiceWorker: The path of the provided scope ‘%1$S’ is not under the max scope allowed ‘%2$S’. Adjust the scope, move the Service Worker script, or use the Service-Worker-Allowed HTTP header to allow the scope. > # LOCALIZATION NOTE: Do not translate "ServiceWorker". %1$S is a stringified numeric HTTP status code like "404" and %2$S is a URL. > ServiceWorkerRegisterNetworkError=Failed to register/update a ServiceWorker: Load failed with status %1$S for script ‘%2$S’. > +# LOCALIZATION NOTE: Do not translate "ServiceWorker". %1$S is a MIME Media Type like "text/plain" and %2$S is a URL. > +ServiceWorkerRegisterMimeTypeError=Failed to register/update a ServiceWorker: Non-JS Content-Type of ‘%1$S’ received for script ‘%2$S’. Similar to the other bug, can we include the scope? Also, I think we should specify exactly what content-types are allowed. Maybe something like: Failed to register/update a ServiceWorker for scope `%1$S`. Bad Content-Type of `%2$S` received for script `%3$S`. Must be `text/javascript`, `application/x-javascript`, or `application/javascript`. ::: dom/workers/ServiceWorkerScriptCache.cpp @@ +803,5 @@ > !mimeType.LowerCaseEqualsLiteral("application/x-javascript") && > !mimeType.LowerCaseEqualsLiteral("application/javascript")) { > + nsXPIDLString message; > + const char16_t* params[] = { NS_ConvertUTF8toUTF16(mimeType).get(), > + PromiseFlatString(mManager->URL()).get() }; Is this usage of temporary return values valid? Since they are not declared as variables on the stack, do they live long enough to pass into FormatLocalizedString()? @@ +807,5 @@ > + PromiseFlatString(mManager->URL()).get() }; > + rv = nsContentUtils::FormatLocalizedString(nsContentUtils::eDOM_PROPERTIES, > + "ServiceWorkerRegisterMimeTypeError", > + params, message); > + NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Failed to format localized string"); Again, might be nice to roll the localization into a shared method in SWM.
Attachment #8768268 -
Flags: review?(bkelly) → review+
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #3) > Is this usage of temporary return values valid? Since they are not declared > as variables on the stack, do they live long enough to pass into > FormatLocalizedString()? Yeah, the temporaries are almost certainly invalid by the time of use... possibly doubly so. I was lining up the types and stopped waaay too early. I'll see if I can make the SWM logging helper function reduce the boilerplate string management required without getting into a template nightmare.
Pushed by bugmail@asutherland.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/b8ca65757e7c report to console when service worker register fails due to mime-type issues. r=bkelly
Assignee | ||
Comment 6•8 years ago
|
||
try job for this was: https://treeherder.mozilla.org/#/jobs?repo=try&revision=86fbfe09ca7b
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b8ca65757e7c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•