1739143 - "JavaScript error: , line 0: uncaught exception: undefined" if "data-l10n-id" points unexistent id

Status: RESOLVED FIXED

(Core :: Internationalization: Localization, enhancement)

Description

[Tooru Fujisawa [:arai]]

Steps to reproduce:

1. Add <label data-l10n-id="non-existent"/> to browser/components/preferences/dialogs/permissions.xhtml file, before </dialog> line
2. ./mach build && ./mach run
3. open about:preferences#privacy
4. Click "Exceptions..." next to "Block pop-up windows" under "Permissions" section

Actual result:
the following is printed to console/terminal, and dialog isn't opened

JavaScript error: , line 0: uncaught exception: undefined

Expected result:
Some more helpful message that says the data-l10n-id is invalid value

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Thank you!
I was able to reproduce and I'd love to improve that DX. Taking.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

:arai - can you verify that in browser console you in fact do see fluent specific warning?

I see:

[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent permissions.xhtml
[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent

I'm looking into why is it showing twice, but I wanted to check with you that you see it at least once :)

The JavaScript error: , line 0: uncaught exception: undefined is just because we don't catch the rejected promise, which we likely should, so I'll work on that.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Hmm, I think the double reporting was a weird artifact, I can't repro anymore.

As for undefined, the reason is that we coalesce a number of promises here: https://searchfox.org/mozilla-central/source/dom/l10n/DocumentL10n.cpp#129

and in case something doesn't translate completely this promise is rejected and uncaughts.

I think in case of DocumentL10n and DOMLocalization we should "silence" the rejection, because it is not actionable. We want to keep it for when TranslateElements or TranslateRoots is called from WebIDL API tho, so I'd like to know how to mark a promise as "ok to be rejected" and not report it.

:smaug - can you advise?

Comment 4

[Tooru Fujisawa [:arai]]

Indeed, I overlooked the warning in browser console, sorry for that!
I was seeing the terminal's stdout/stderr, and then checked the browser console also has the error, but didn't notice the warnings.

what I get in the browser console is:

[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent permissions.xhtml
Uncaught (in promise) undefined
uncaught exception: undefined

and what I get in terminal is:

JavaScript error: , line 0: uncaught exception: undefined
JavaScript error: , line 0: uncaught exception: undefined

for each click.

so, the expected behavior about the helpful information is already there in browser console
just that there's extra error, and only that extra error is printed to terminal that's slightly confusing.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

gotcha, yes! I'd like to fix that:

1. Stop showing the uncaught promise (question to :smaug)
2. Show the error in the terminal alongside console

For the (2) - :emilio, here's what I'm using now - https://searchfox.org/mozilla-central/source/intl/l10n/Localization.cpp#56-58 - is there a way to make it also dump to terminal alongside console?

Comment 6

[Emilio Cobos Álvarez (:emilio)]

printf_stderr("%s\n", error.get()); or so?

Comment 7

[Tooru Fujisawa [:arai]]

fwiw, JavaScript error is printed to stderr here:
https://searchfox.org/mozilla-central/rev/d2f8488b6a704443a5c5bfc6d2878171b5f0d393/js/xpconnect/src/nsXPConnect.cpp#273-294
during printing to console:
https://searchfox.org/mozilla-central/rev/d2f8488b6a704443a5c5bfc6d2878171b5f0d393/js/xpconnect/src/nsXPConnect.cpp#311

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #3)

I think in case of DocumentL10n and DOMLocalization we should "silence" the rejection, because it is not actionable. We want to keep it for when TranslateElements or TranslateRoots is called from WebIDL API tho, so I'd like to know how to mark a promise as "ok to be rejected" and not report it.

I'm not aware of such. Or do you perhaps mean calling StealExceptionFromJSContext on some ErrorResult and explicitly reject a promise?

Comment 9

[Tooru Fujisawa [:arai]]

I guess the undefined is from here?
https://searchfox.org/mozilla-central/rev/6c8d325e61b0b445ed2e04899da38c3a4c266cba/dom/l10n/DocumentL10n.cpp#77

what's the expected consumer of the promise fulfilled/rejected there?
isn't the consumer supposed to handle the rejection somehow?

if we're completely not interested into the rejection, we could just call MaybeResolveWithUndefined() instead,
but maybe that's wrong way to go...

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1739143 - Capture DOM L10n initial translation and mutations rejections and report to console. r?nordzilla

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f859ea95ec6
Capture DOM L10n initial translation and mutations rejections and report to console. r=nordzilla

Comment 12

[Atila Butkovits]

Backed out for causing failures at document_l10n/non-system-principal/browser_resource_uri.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/4f859ea95ec672b38068383ddacc19d9eb895cff

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=BJFf-fUQSXiyVC-5nHfzJw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=4f859ea95ec672b38068383ddacc19d9eb895cff

Failure log:
-https://treeherder.mozilla.org/logviewer?job_id=357576579&repo=autoland&lineNumber=10930
-https://treeherder.mozilla.org/logviewer?job_id=357576669&repo=autoland&lineNumber=3065

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c53702d3deac
Backed out changeset 4f859ea95ec6 for causing failures at document_l10n/non-system-principal/browser_resource_uri.js. CLOSED TREE

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0d60e019527
Capture DOM L10n initial translation and mutations rejections and report to console. r=nordzilla

Comment 15

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/e0d60e019527

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Low prio but when you have a moment, can you verify that the DX is now matching your expectation?

Comment 17

[Tooru Fujisawa [:arai]]

Yes, it works perfectly!

I get the following in the terminal:

[fluent] Missing message in locale en-US: non-existent
[fluent] Couldn't find a message: non-existent
[dom/l10n] Could not complete initial document translation.

and the following in the browser console:

[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent permissions.xhtml
[dom/l10n] Could not complete initial document translation. permissions.xhtml

and the dialog opens, with the label with non-existent data-l10n-id untranslated.