Closed Bug 408697 Opened 14 years ago Closed 14 years ago

Certificate Exception dialog logs scary looking exception, even though nothing's wrong

Categories

(Core :: Security: PSM, defect)

defect
Not set
trivial

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: sgautherie, Assigned: johnath)

References

Details

(Keywords: polish)

Attachments

(1 file)

Moved from bug 408432 comment 11:

{{{
Johnathan Nightingale   2007-12-17 08:55:45 PST

> > the JS Error Message is Error: [Exception... "Component returned failure code:
> > 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.send]"  nsresult: "0x80004005
> > (NS_ERROR_FAILURE)"  location: "JS frame ::
> > chrome://pippki/content/exceptionDialog.js :: checkCert :: line 151"  data: no]
> > Source File: chrome://pippki/content/exceptionDialog.js
> > Line: 157

The exception is behaving as-designed (strange as that may seem).  See here:

http://mxr.mozilla.org/mozilla/source/security/manager/pki/resources/content/exceptionDialog.js#157

Though, if you found it really confusing, you might file a bug on improving the
log message there, to say something about it being expected - or maybe logging
it differently altogether?
}}}

Code (from bug 387181) is
{{
 153   } catch (e) {
 154     // We *expect* exceptions if there are problems with the certificate
 155     // presented by the site.  Log it, just in case, but we can proceed here,
 156     // with appropriate sanity checks
 157     Components.utils.reportError(e);
 158   } finally {
}}
(In reply to comment #0)
> Code (from bug 387181) is

Rather bug 387480.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b5pre) Gecko/2008031801 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

(Bug still there.)
Depends on: 387480
No longer depends on: 387181
Flags: blocking1.9?
I don't think this blocks release, but it could be confusing, and it is very low-hanging fruit.
Assignee: kengert → johnath
Status: NEW → ASSIGNED
Attachment #310453 - Flags: review?(kengert)
Summary: "Error / Exception / NS_ERROR_FAILURE / nsIXMLHttpRequest.send / exceptionDialog.js :: checkCert :: line 151 and 157": make it clear that it's not a bug (to be reported). → Certificate Exception dialog logs scary looking exception, even though nothing's wrong
Comment on attachment 310453 [details] [diff] [review]
More verbose log message

Now, that looks a little too verbose to me,
but I'd rather have too much than not enough.

I wonder if we could/want to report this as a Message (or may be Warning) rather than an Error ?
Moreover/Or, I wonder if that could/should be made "debug build only" ?
(In reply to comment #4)
> (From update of attachment 310453 [details] [diff] [review])
> Now, that looks a little too verbose to me,
> but I'd rather have too much than not enough.
> 
> I wonder if we could/want to report this as a Message (or may be Warning)
> rather than an Error ?
> Moreover/Or, I wonder if that could/should be made "debug build only" ?

We don't want this for debug only, since the reason for logging it in the first place is that we think it's unlikely, but not impossible, that a "real" exception is being thrown here, and we don't want to lose all hope of catching that, if a user stumbles on to it.

We could change it to a warning or info message instead of error, using nsIConsoleService - I'm not sure what Kai prefers here, but I'll leave it to him, since he has a broader knowledge of how PSM reports things to the console.

-'ing, but wanted1.9.0.x+.  Would be a nice to have, request approval once patch completed.  Still, won't block the release.
Flags: wanted1.9.0.x+
Flags: blocking1.9?
Flags: blocking1.9-
Comment on attachment 310453 [details] [diff] [review]
More verbose log message

r=kengert, although this message will never get localized...
Attachment #310453 - Flags: review?(kengert) → review+
Attachment #310453 - Flags: approval1.9?
(In reply to comment #7)
> (From update of attachment 310453 [details] [diff] [review])
> r=kengert, although this message will never get localized...

For now, that may help to get it in for Gecko 1.9, wrt l10n.
Should I file a followup bug for Gecko 2.0 ?
(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 310453 [details] [diff] [review] [details])
> > r=kengert, although this message will never get localized...
> 
> For now, that may help to get it in for Gecko 1.9, wrt l10n.
> Should I file a followup bug for Gecko 2.0 ?

I agree - this is not worth the late-l10n hit, but I have no problem with it being localized in principle, unless we have a policy against localizing error messages.  My advice would be to file a followup, cc me, and maybe even tag the status whiteboard as "[good first bug]".  If some new volunteer doesn't get to it before I do, it should be a simple matter to add it to the right properties file, and retrieve it in js instead of hard coding.
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (From update of attachment 310453 [details] [diff] [review] [details] [details])
> > > r=kengert, although this message will never get localized...
> > 
> > For now, that may help to get it in for Gecko 1.9, wrt l10n.
> > Should I file a followup bug for Gecko 2.0 ?
> 
> I agree - this is not worth the late-l10n hit, but I have no problem with it
> being localized in principle, unless we have a policy against localizing error
> messages.  My advice would be to file a followup, cc me, and maybe even tag the
> status whiteboard as "[good first bug]".  If some new volunteer doesn't get to
> it before I do, it should be a simple matter to add it to the right properties
> file, and retrieve it in js instead of hard coding.
> 

The followup bug might also want to address comment 4, where you talk about logging it at a different severity
Comment on attachment 310453 [details] [diff] [review]
More verbose log message

a1.9+=damons
Attachment #310453 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in security/manager/pki/resources/content/exceptionDialog.js;
/cvsroot/mozilla/security/manager/pki/resources/content/exceptionDialog.js,v  <--  exceptionDialog.js
new revision: 1.9; previous revision: 1.8
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008040715 SeaMonkey/2.0a1pre] (SEA-WIN32-TBOX-trunk) (W2Ksp4)

With <https://gmail.com/>,
{{
Error: Attempted to connect to a site with a bad certificate in the add exception dialog. This results in a (mostly harmless) exception being thrown. Logged for information purposes only: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.send]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://pippki/content/exceptionDialog.js :: checkCert :: line 151"  data: no]
Source File: chrome://pippki/content/exceptionDialog.js
Line: 159
}}

Although, whereas the source code is nicely on 3 lines,
the resulting "one endless line" console text is "difficult" to read. :-/

V.Fixed
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9
Blocks: 427674
Flags: wanted1.9.0.x+
You need to log in before you can comment on or make changes to this bug.