Closed Bug 801663 Opened 7 years ago Closed 7 years ago

Unify error fallbacking

Categories

(L20n :: General, defect, P1)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Current implementation of L20n.js behaves differently in each of these scenarios:

1) Developer didn't add an import (and in result some entities don't compile)
2) Localizer did not add an @import
3) An entity has not been added to the LOL by the localizer
4) An entity has not been parsed due to an error

I believe that we should follow CSS and revive as early as possible from all scenarios.
In particular:

1) Any missing entity requested directly by the developer should result in an Exception/Error emitted
2) Any entity that did not compile due to missing dependency or a bug should result in an Exception/Error
3) Any missing import from the developer or localizer should be debuggable but the library should operate as if it was there
4) Any parser error should result in an Exception/Error emitted

5) If for any reason an entity cannot be returned, the library should create a fallback context and try to get the entity from there. After the operation the library should get back to use the main context.
6) If no context is available (e.g. all fallbacks failed to return the entity) return a message like "Error: Could not return EntityID"
7) If a fallback has been used emit Error
8) If a fallback has been used on document l20n context emit LocalizationFallback
9) If no fallback remain and the document has not been fully localized emit LocalizationFailed

Optionally:
10) If majority of entities in a HTML file were localized using a fallback, then consider retranslating the few that were localized using the main context in order to provide consistency.
Oh this is fun.

:)
Assignee: nobody → gandalf
Priority: -- → P1
Target Milestone: --- → 1.0
Blocks: 801665
Blocks: 803399
Flags: needinfo?(l10n)
For most of these, I wouldn't go beyond logging, possibly behind a pref so that it's only visible in "debugging", whatever that means for an API.

Programmer-visible errors should only be issued if a string can't be returned at all.
Flags: needinfo?(l10n)
Flags: needinfo?(gandalf)
Blocks: 809303
My thinking got much further now, so here's an updated proposal:

We have four scenarios:

1) entity value has been returned for a preferred locale (or without locales) without any problem.
2) entity value has been returned with a fallback locale.
3) no valid value for an entity has been computed but we have the complexstring for at least one language. 
4) the entity could not been retrieved for any locale from the list.
  
1) We return the value for the preferred locale
2) We return the value for the first locale in the fallback that properly computes
3) We return the first value for this entity for the first locale that can be pulled, partially evaluated (bug 803399)
4) We return the entity ID

Now, I'd like to introduce three debug scenarios:
1) user mode
2) localizer
3) l20n lib user, developer
4) l20n lib developer

In user mode we don't report anything else, but we emit exceptions
In localizer mode we report localization bugs and stop on any errors that require fallback to another locale
In l20n lib user, developer mode, we emit errors and and exceptions
In l20n lib dev debug mode we report everything (all debug info).


How does this sound?
Flags: needinfo?(gandalf)
Flags: needinfo?(stas)
Blocks: 810455
After a long conversation with Pike and Stas, this is the new proposal:

We extend L20n.js with one L20nContext.settings addition in form of a flag "dontFallback" which makes the L20nContext:
 - not fallback on error
 - throw exception on any error in ctx.get

We then adjust l20n-xml code to:
 - switch this flag on on localizationReady when it begins localizing nodes
 - when it encounters a node that is missing (and throws exception) it ignites fallback context loading asynchronously and add this node to the list of nodes that will be localized when fallback context is ready
 - it continues localizing nodes, and if it encounters more nodes that cannot be translated it adds them to that list
 - when subcontext is loaded it localizes those that were waiting
 - it fires DocumentLocalized only when all nodes are localized
 - on firing DocumentLocalized it switches off the dontFallback flag to off

 - if DocumentLocalized has loaded and the developer programmatically asks for an entity that we don't have, we load fallback context synchronously.

 - before DocumentLocalized we encourage to use getAsync or prepare for get throwing
I've been thinking about this more tonight and I'd like to propose a small change to the spec from comment 4.

Instead of a flag set on context that significantly changes its behavior and makes it unclear what the result of "ctx.get('missingKey');" would be, I suggest we have "ctx.getOrError(key)" which is only synchronous and throws on error instead of emitting errors.

As Pike pointed out in a conversation, getAsync should always create the subctx asynchronously, because, well, it's asynchronous so it's already waiting for something.
Flags: needinfo?(stas)
Depends on: 802845
Flags: blocking-parkcity+
followup in bug 838166
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.