1027157 - Some warnings show up as "errors" in the console due to using JSEXN_TYPEERROR or JSEXN_SYNTAXERROR, etc

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: example of confusion

Despite each of these messages always being logged as a warning, they use error types rather than warning types. We should make them into proper warnings because right now they look like full on errors and it is confusing because they don't actually halt JS execution.

http://dxr.mozilla.org/mozilla-central/source/js/src/js.msg#263
http://dxr.mozilla.org/mozilla-central/source/js/src/js.msg#356
http://dxr.mozilla.org/mozilla-central/source/js/src/js.msg#400

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Jeff, what's the best way to do this? Can we just create JSEXN_SYNTAXWARN, etc or something like that?

Comment 2

[Jeff Walden [:Waldo]]

So I think just changing the error type to JSEXN_NONE is the short-term fix here.  Longer-term, we need to actually have a warning mechanism not conflated with the error/exception mechanism.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: warnings-not-errors.patch

This just switches to JSEXN_NONE.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://tbpl.mozilla.org/?tree=Try&rev=e8b7d20e7e6b

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/57d65c06db19

Comment 6

[Chris Peterson [:cpeterson]]

Nick: do you think we should uplift this fix to Aurora 32 or even Beta 31? This is a small fix and the bug  may confuse to web developers, especially when they are using third-party JS libraries that cause these "errors".

These warnings were introduced in Firefox 30 (bug 948227), so fixing Beta 31 would soon correct this confusion about bogus TypeErrors.

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Chris Peterson (:cpeterson) from comment #6)
> Nick: do you think we should uplift this fix to Aurora 32 or even Beta 31?
> This is a small fix and the bug  may confuse to web developers, especially
> when they are using third-party JS libraries that cause these "errors".
> 
> These warnings were introduced in Firefox 30 (bug 948227), so fixing Beta 31
> would soon correct this confusion about bogus TypeErrors.

Yes I do.

I was going to wait for the patch to land on m-c before requesting approval. Pretty simple patch though, definitely shouldn't break anything.

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/57d65c06db19

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8443096 [details] [diff] [review]
warnings-not-errors.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 948227, among others
[User impact if declined]: Warnings show up as errors in the console, which is lying to the user and can be confusing when you notice your JS execution doesn't actually halt and no exception gets thrown.
[Describe test coverage new/current, TBPL]: Has been on m-c for a while now, no issues.
[Risks and why]: Very little risk. Literally just changing JSEXN_SOMEKINDOFERROR to JSEXN_NONE so that it isn't reported as an error.
[String/UUID change made/needed]: None.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

checkin-needed is not needed. approval-mozilla-* is enough.

Comment 11

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

My bad! Thanks!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/93c35d7aec1f
https://hg.mozilla.org/releases/mozilla-beta/rev/aa2a7bcabf52