loglevel doesn't play nice with modernizr on ie8

[kraf]

modernizr introduces the bind method to Function.prototype in IE8 which then throws errors when loglevel library is loaded. this seems to happen because in IE8 methods like console.log and console.error have types object (i.e. "typeof console.log === 'object' and typeof console.log !== 'function').

[pimterry]

Good spot, thanks for this!

I'll try and take a look to get a fix in within the next few weeks for v0.4. It sounds like you know what's going on though, and if you're keen you'd be very welcome to fix it and send a pull request!

[kraf]

can't do it at work, but i can get around to it next week if you're not pressed for time

[pimterry]

Fine by me, I'm not in any particular hurry on this (heh, I'm not using modernizr 😄) and I'm actually on holiday for the next week and a bit anyway.

Take your time, but do shout if you're not keen or you don't have time or something please so I can make sure I get around to doing it myself.

[pimterry]

Hi @kraf: are you still looking at this?

[kraf]

yes i am. i'm sorry for the delay, i'm not a windows user and i rarely find myself in a room with the good old IE8.

i know where to fix it, but i'm not completely sure how. i will have an IE8 to play with on sunday to test stuff out, but i'd love to get your take on this.

this method needs changing:

function boundToConsole(console, methodName) {
    var method = console[methodName];
    if (method.bind === undefined) {
        if (Function.prototype.bind === undefined) {
            return function() {
                method.apply(console, arguments);
            };
        } else {
            return Function.prototype.bind.call(console[methodName], console);
        }
    } else {
        return console[methodName].bind(console);
    }
}

basically we need to either a) test if console[methodName] is of type 'object' and if it supports apply

if(typeof console[methodName] === 'object' && console[methodName].apply)

and then just hope this works everywhere or b) specifically test for Internet Explorer 8 (and lower i suppose) and
then just do the

return function() {
    method.apply(console, arguments);
};

but i don't know the best practice way to test for IE8.

what do you think?

[pimterry]

@kraf Sorry, I did take a look at that, but it's difficult to work out exactly what's going on without a console to play with it, and I don't have IE8 to hand. As a rule though, I'd avoid trying to detect specific browsers and instead work out what exactly's actually happening (i.e. Function.prototype.bind.call(console[methodName], console) explodes, I assume), and then try and explicitly handle that with something sensible. Likely to be less brittle.

Did you make any progress on this in the end? No worries if not, I've got time to take a closer look next week I think, so I can sort it then. It'd be good to get this finished up so I can do a big browser test for compatibility and hopefully sort the 0.4 release within the next few weeks.

[kraf]

Hey, you are right, that's where it breaks. I agree with you when it comes to testing for a specific browser, this will only get you in hell's kitchen.

I didn't look into it any more after the last post, but what i can definitely tell you is that testing if console.log is an object or a function must be the way to go. Frankly, in the end the only reason why I didn't just try this out myself was because I didn't get how to run the tests right away and i never used Travis.

console.log, console.error and console.info exist on IE8 (console.debug is undefined), and are all of type "object". I checked in Chrome and Firefox and there they are all of type "function", which is what you'd expect.

[overdub60]

The issue still exists for me here with IE8 on WinXP. This is the full error:

Message: Exception thrown and not caught
Line: 129
Character: 21
Code: 0
URI: xxxx/lib/loglevel.js

I'm also using modernizr. Any ideas for a fix?
Thanks.

[pimterry]

Hmm, yes, sorry, I fixed an IE8 bug and assumed it'd also solved this. There's now a further fix here that I've properly tested against Modernizr, and seems to work for me. If you could check it's good for you (either @overdub60 or @kraf) that'd be great.

I'll do a bit more testing myself, and try to release this as 0.4.1 within the next few days.

[overdub60]

Thanks for attempting to fix this. Unfortunately, I'm still getting the same error, same line causing the issue throw "No console available for logging";.

[pimterry]

Woah; "no console available for logging" is actually a different error entirely, and is supposed to happen if you try to change the log level in an environment where logging is completely unavailable (see the setLevel section in the docs). In IE (< 9, I think? maybe < 10) the console is not available until you've opened developer tools, and then is available in that tab for the rest of the session. That isn't related to Modernizr (afaik).

The fix above is actually for an issue where it doesn't work if you do have the console open, because the console object then exists, but Modernizr creates a slightly non-conformant .bind() shim, and that breaks things. That's the original error that @kraf reported.

This exception is intentionally thrown in this scenario because setLevel is intended to be used manually, really, to enable more detailed logging for you development sessions and similar. It needs to throw an error rather than fail silently because otherwise if you try to manually run it in a browser where it doesn't work, silent failure means there's no way to work out why your logging is still not happening.

This is one bit of the API that is still a little debatable though, so if you've got strong views on changing this I'd be keen to hear them!

[overdub60]

Oh, sorry for the confusion. I got the impression that we were talking about the same error. Glad that's cleared up now. :-)

What I didn't expect is that loglevel behaves differently in IE compared to Chrome for example. In Chrome, even if I don't have the console open, no error is thrown. In IE, my app completely breaks because of the exception when I don't have the console open. I would expect loglevel to behave the same in all environments and I personally much prefer the way it's handled in Chrome. I won't expect my logging to work (i.e. be visible) if I don't have a console open, I don't need an exception for that. When my console is open, I want to see logging, when it's closed of course I won't see anything. These are just my 2 cents. I would be interested in hearing your opinion on this.

[pimterry]

Right, yes, sorry. @overdub60 that was definitely expected and correct behaviour. There's not a lot you can do with the basics of it; it's not just that the logging doesn't go anywhere when the console's closed, it's that until you open it IE doesn't build any of the environment for it at all. Oh IE.

Loglevel will work totally fine for actual logging methods (.trace(), .debug(), .error(), etc) -- they'll handle console present or not correctly and never throw exceptions -- but setLevel() intentionally loudly fails because you're only really supposed to actually change the log level manually when you're investigating things, and you definitely don't want to be debugging, enable logging, and silently see nothing with no explanation.

Fortunately though I've found a third option that's now in trunk; I've switched from throwing errors to just returning them. This should mean if you're running setLevel() manually you get clear responses in the console explaining any issues, and if you're not then it'll just quietly fail to log for a while. That's not really an acceptable fix in itself, but I've managed to tweak the fallback handlers so that if you try and log later and IE has then got around to adding a console object, it'll start logging more properly from that point forwards, which should hopefully make this basically work perfectly transparently in IE, I'm fairly hopeful.

TL;DR: this should mean your use case now works properly.

I'd still lightly caution against embedded logging level changes in your application, but they will now act sensibly in that situation, and still give feedback when used manually. Give it a try on master if you get a minute, and I'll get a proper 0.5.0 release out shortly once I've done some testing.

[pimterry]

Both the original issue here and overdub60's concerns are solved as part of v0.5.0, which I've just released. I'll close this ticket off now, thanks for your input everybody!