The default bug view has changed. See this FAQ.

Descriptive error messages in l10n.js

RESOLVED FIXED

Status

Firefox OS
Gaia::L10n
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: timdream, Assigned: stas)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

The error thrown in l10n.js is not descriptive enough in adb log. I spent non-trivial time looking for the cause here...

I would like to append a "l10n.js" label in the message.
Summary: Descript error messages in l10n.js → Descriptive error messages in l10n.js
Created attachment 8425383 [details] [review]
mozilla-b2g:master PR#19427
Attachment #8425383 - Flags: review?(gandalf)
Tim: which message was confusing?

I'm ok with the change, but I'm wondering if we can unify the places where we emit things to adb log.

Stas, what do you think?
Flags: needinfo?(timdream)
Flags: needinfo?(stas)
I am getting message like

E/GeckoConsole( 1527): [JavaScript Error: "ContextError: Context not ready"]

and no lines before or after, when working on bug 1013155. Took me a while to figure out where that is because the object referenced to the remote input field in the keyboard scope is called InputContext.
Flags: needinfo?(timdream)
Ah, good catch. Yeah, we should make it more descriptive. Also, I see a few of those bugs and would love to start hunting them down once we get bug 1000806 fixed because this bug means that there's code that tries to access l10n resources without waiting for them to be ready.

Maybe we can get this message more descriptive in particular - basically saying what I wrote above.
Yeah sure, no worries. Do you guys want to pick up this bug instead of taking my patch?
(Assignee)

Comment 6

3 years ago
Created attachment 8427764 [details] [diff] [review]
L10nError

Thanks for the report and for the patch, Tim!  I'll be happy to take this bug over.

We used to have more descriptive error classes, but due to memory usage constraints, we remove them.  ContextError was the only one that we left, and I agree that the name might not be immediately helpful.

I tried a different approach:  remove ContextError and replace it with L10nError which is available to all of l10n.js.

Unexpected errors now look like this in adb logcat:

  E/GeckoConsole(  974): [JavaScript Error: "L10nError: Context not ready"]
Assignee: timdream → stas
Attachment #8425383 - Attachment is obsolete: true
Attachment #8425383 - Flags: review?(gandalf)
Attachment #8427764 - Flags: review?(gandalf)
Flags: needinfo?(stas)
Comment on attachment 8427764 [details] [diff] [review]
L10nError

Review of attachment 8427764 [details] [diff] [review]:
-----------------------------------------------------------------

can you do a quick b2gperf check on this?
(In reply to Staś Małolepszy :stas from comment #6)
>   E/GeckoConsole(  974): [JavaScript Error: "L10nError: Context not ready"]

Yeah that's good. Thanks!
(Assignee)

Comment 9

3 years ago
(In reply to Zibi Braniecki [:gandalf] from comment #7)

> can you do a quick b2gperf check on this?

App       Δ median  Δ mean  Sig?
--------  --------  ------  ----
Browser          1       7      
Calendar        -5     -17      
Camera           1      -1      
Clock            7       3      
Contacts        14      38      
Email           -7      21      
Messages         6      19      
Music           -1       5      
Phone           -3       3      
Settings         5     -24      
Usage            5      18      
Video            2       5      
Gallery          4      43      

No statistically significant differences.  Were you suspecting any?
Comment on attachment 8427764 [details] [diff] [review]
L10nError

Review of attachment 8427764 [details] [diff] [review]:
-----------------------------------------------------------------

Yes. Glad to see I was wrong :) r+
Attachment #8427764 - Flags: review?(gandalf) → review+
(Assignee)

Comment 11

3 years ago
Commit: https://github.com/mozilla-b2g/gaia/commit/50991928704bfa5569b3a6a3ef12338fc4573b6c
Merged: https://github.com/mozilla-b2g/gaia/commit/8d8d34b01bec60940664bca8319413f84b1e51ea

L20n.js: https://github.com/l20n/l20n.js/commit/1c9fa39943def7c29ab5cbf44c58032b22024e7a
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.