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

VERIFIED FIXED in mozilla1.8.1

Status

()

Core
JavaScript Engine
P3
minor
VERIFIED FIXED
12 years ago
10 years ago

People

(Reporter: a-higuti, Assigned: a-higuti)

Tracking

({verified1.8.1})

Trunk
mozilla1.8.1
verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
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);
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Target Milestone: --- → mozilla1.8.1
Created attachment 237060 [details] [diff] [review]
Tweaked patch
Attachment #237060 - Flags: review?(brendan)

Updated

12 years ago
Assignee: general → a-higuti

Updated

12 years ago
Flags: blocking1.8.1?
Comment on attachment 237060 [details] [diff] [review]
Tweaked patch

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

/be
Attachment #237060 - Flags: review?(brendan) → review+

Comment 4

12 years ago
It looks like this bug is a duplicate of bug 345734
*** Bug 345734 has been marked as a duplicate of this bug. ***
(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

12 years ago
>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?
Flags: blocking1.8.1? → blocking1.8.1+
I just checked this into the trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
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.
Attachment #237060 - Flags: approval1.8.1?

Comment 10

12 years ago
Comment on attachment 237060 [details] [diff] [review]
Tweaked patch

a=schrep for drivers.
Attachment #237060 - Flags: approval1.8.1? → approval1.8.1+

Comment 11

12 years ago
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
Flags: in-testsuite+

Comment 12

12 years ago
verified fixed 1.9 20060909 windows/mac*/linux
Status: RESOLVED → VERIFIED

Updated

12 years ago
Whiteboard: [checkin needed (1.8 branch)]

Comment 13

12 years ago
can we get this landed on 1.8.1 branch and mark fixed1.8.1.
I am a CVS branch monkey.

/be
Keywords: fixed1.8.1

Comment 15

12 years ago
verified fixed 1.8 20060914 windows/linux 1.9 20060914 windows/mac*/linux
Keywords: fixed1.8.1 → verified1.8.1
Whiteboard: [checkin needed (1.8 branch)]

Updated

10 years ago
Duplicate of this bug: 345734
You need to log in before you can comment on or make changes to this bug.