166743 - Error.prototype.stack is still causing headaches

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Robert Ginda]

toString()ing parameters for Error.prototype.stack is causing headaches again. 
See bug 166420 and bug 69850 for examples.

Calling toString on arbitrary objects in an error condition just doesn't seem
like a good thing.  I'd suggest that the best we can do is append the string
[Object] (or something like that) wherever we find an object parameter.

Comment 1

[Robert Ginda]

Uh, that should have been bug 165179, not bug 69850.

Comment 2

[Phil Schwartau]

This may be related to a bug I filed: bug 157474
"Error.stack no longer quotes string arguments to functions"

Comment 3

[Robert Ginda]

Attachment: hacked patch

If I apply this patch I no longer see the long pause before reporting an error
in Venkman.  I assume this fixes the crash too, but seeing as how I couldn't
crash in a debug build, I don't know for sure.

I imagine we don't actually want to JS_NewStringCopyZ here (or maybe just for
1.2a.)

Comment 4

[Brendan Eich [:brendan]]

Attachment: less hacked fix, plus a fix for failure to report no-such-file from the shell

The fix to return false (meaning "please report this error, I can't make an
exception object for it") from js_ErrorToException mirrors logic used in
jsapi.c to call js_ReportUncaughtException when unwinding to an API caller when
there are no active frames on cx.

I rearranged the logic in rginda's patch to avoid redundant tests for primitive
type values.

/be

Comment 5

[Brendan Eich [:brendan]]

Cc'ing more JS buddies and drivers.  Rob, can you r= my patch?  Phil, testing
would be good -- this should get into the frozen 1.2alpha trunk by tomorrow. 
Thanks,

/be

Comment 6

[Robert Ginda]

Comment on attachment 98011 [details] [diff] [review]
less hacked fix, plus a fix for failure to report no-such-file from the shell

r=rginda

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 98011 [details] [diff] [review]
less hacked fix, plus a fix for failure to report no-such-file from the shell

a=[e-mail]

Comment 8

[Brendan Eich [:brendan]]

Fixed.

/be

Comment 9

[Phil Schwartau]

Marking Verified Fixed.

Please also see bug 157474 and bug 166420. In particular, 
http://bugzilla.mozilla.org/show_bug.cgi?id=166420#c9
shows the effects of this fix.

Rob: if the fix hasn't addressed your concerns, please reopen; thx -