1494611 - Marionette logs debug/trace lines when "marionette.log.level" uses all capitalized letters

Status: RESOLVED WONTFIX

(Remote Protocol :: Marionette, enhancement, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Setting the "marionette.log.level" preference to eg. "INFO" causes Marionette to log all debug and trace output.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The problem here is that we delegate the retrieval of the log level to `logger.manageLevelFromPref(PREF_LOG_LEVEL)`. And that doesn't care about broken capitalization. It just picks what it gets, or uses the `All` log level:

https://dxr.mozilla.org/mozilla-central/rev/024521c589d28744b5c4a6c01127fa35edef2c53/toolkit/modules/Log.jsm#213-238

We could enhance the logger for that particular situation, and allow it to fallback to the `Log.Level.Numbers` property, which has all capitalized letters. But I might not wanna touch that.

Alternatively we have our `logLevel()` method of Marionette prefs, which currently is not used. As such we could just fix the level in the preference so that Log.jsm's preference observer will pick it up. Not sure if that is ideal either.

Basically the `logLevel()` getter which lowercases all values is currently unused in any of the modules. And as such any level not starting with a capitalized letter, and having lowercase letters following will cause everything to be logged.

Andreas, which path would you take? Should we just leave it as what we have right now? Clients would have to be fixed to send the correct value for the log level. Personally I would prefer this solution.

Comment 2

[Andreas Tolfsen ❲:ato❳]

The presumption was always that the input to marionette.log.level
would be in the correct format and that internal consumers would
use fail-safe MarionettePrefs.logLevel to get at the log level.

I don’t think there’s anything we can do here without modifying
Log.repository.manageLevelFromPref, and from past conversations
with mossop, it would be an unwelcome change to ignore casing in
Log.jsm.

One slightly hacky option might be to read MarionettePrefs.logLevel
before calling manageLevelFromPref, and then write the value again.
But that might cause race conditions witht he first loggers that
are returned, and causes unnecessary writes to the preference
registry.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, good to see that we have the same feeling here. Lets leave the code as is and close this bug as wontfix.