350650 - js reports "uncaught exception: unknown (can't convert to string)"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[a-higuti]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6 XpcomViewer/0.9
Build Identifier: 

When an exception is thrown in a js code and it is not caught, js reports
'uncaught exception: ...' message. The problem is, if the exception is an
object and has toString() method written in javascript, js fails to convert
it to string and reports "uncaught exception: unknown (can't convert
to string)".

Reproducible: Always

Steps to Reproduce:
1. get the source from CVS repository
2. build js
3. execute the following js code

function exc() { this.toString = function() { return "EXC"; } }
throw new exc();


Actual Results:  
$ ./js excerr.js
uncaught exception: unknown (can't convert to string)

Expected Results:  
$ ./js excerr.js
uncaught exception: EXC


js_Interpret fails if cx->throwing is true. i think we must clear
cx->throwing before invoking js_ValueToString(cx, exn).

--- js/src/jsexn.c.org  2006-08-30 10:53:40.000000000 +0900
+++ js/src/jsexn.c      2006-08-30 10:54:37.000000000 +0900
@@ -1100,6 +1100,8 @@
     if (!JS_GetPendingException(cx, &exn))
         return JS_FALSE;

+    JS_ClearPendingException(cx);
+
     /*
      * Because js_ValueToString below could error and an exception object
      * could become unrooted, we must root exnObject.  Later, if exnObject is
@@ -1179,7 +1181,6 @@
         js_ReportErrorAgain(cx, bytes, reportp);
     }

-    JS_ClearPendingException(cx);
 out:
     if (exnObject)
         js_FreeStack(cx, mark);

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

This is indeed a bug, I've tweaked your patch slightly to clear the pending exception after we've rooted it in the allocated stack, but other than that, it looks good.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Tweaked patch

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 237060 [details] [diff] [review]
Tweaked patch

Great -- thanks for the patch, and thanks to mrbkap for helping get it in.

/be

Comment 4

[Mike Moening]

It looks like this bug is a duplicate of bug 345734

Comment 5

[Brendan Eich [:brendan]]

*** Bug 345734 has been marked as a duplicate of this bug. ***

Comment 6

[Brendan Eich [:brendan]]

(In reply to comment #4)
> It looks like this bug is a duplicate of bug 345734

Thanks for naming that bug, I knew you knew it.  Can you vouch for the patch here?  Thanks again,

/be

Comment 7

[Mike Moening]

>Can you vouch for the patch here?
I know my original problem involved an object too.
Not sure if it was **exactly** the same.  
My ability to reproduce this was not 100% due to the other weird crashes and corruptions I kept bumping into.

There was another entry in the js-engine newsgroup on 8/25 that referred to this problem specifically.

I gave the guy a hack suggestion that worked for me. Maybe take a look at what I wrote and see if you can reconcile the two?

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

I just checked this into the trunk.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 237060 [details] [diff] [review]
Tweaked patch

This will make error output better for uncaught exceptions, as exception objects implemented in JavaScript will now function correctly.

Comment 10

[Mike Schroepfer]

Comment on attachment 237060 [details] [diff] [review]
Tweaked patch

a=schrep for drivers.

Comment 11

[Bob Clary [:bc] (inactive)]

Checking in regress-350650-n.js;
/cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-350650-n.js,v  <--  regress-350650-n.js
initial revision: 1.1

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060909 windows/mac*/linux

Comment 13

[Mike Schroepfer]

can we get this landed on 1.8.1 branch and mark fixed1.8.1.

Comment 14

[Brendan Eich [:brendan]]

I am a CVS branch monkey.

/be

Comment 15

[Bob Clary [:bc] (inactive)]

verified fixed 1.8 20060914 windows/linux 1.9 20060914 windows/mac*/linux