1071870 - Log.Level.All doesn't work with some /services/ modules since they assume valid log levels are truthy

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Matthew N. [:MattN]]

Attachment: v.1 Check for undefined instead of truthiness since All === 0

Log.jsm's Log.Level.All === 0 which is kinda unfortunate as it leads to bugs like this one. We have some modules that were doing the following and therefore fail when Log.Level.All is specified:
  log.level = Log.Level[level] || Log.Level.Error;

Comment 1

[Matthew N. [:MattN]]

Attachment: v.2 Fix the problem at the source (All === 0)

As discussed on IRC with markh and Unfocused, switching All to -1 will get rid of the footgun.

TBH I'm not sure why there are two mapping from Level to number but I think this should be fine.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8494227 [details] [diff] [review]
v.2 Fix the problem at the source (All === 0)

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

LGTM, although I'd be inclined to add a comment to the first map to briefly say we don't want .All to be falsey.

Comment 3

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/fx-team/rev/56a690a7c1bc

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/56a690a7c1bc