626909 - jsd.onError and jsd.onDebug fail

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[John J. Barton]

jsd.onError is no longer called for JavaScript syntax errors.

jsd.onDebug is no longer called when jsd.onError returns false.

Testcase next.

Comment 1

[John J. Barton]

https://fbug.googlecode.com/svn/tests/mozilla/jsdOnDebug is an simple bootstrap
addon that writes to window.dump.

You will need to enable
browser.dom.window.dump.enabled true
And open Error Console and an os console to see window.dump

On start up you should see:
____________ load into window ___________________
firebug-service.activation begin jsd.asyncOn 1295401372217
____________ done load into window ___________________
firebug-service.activation now we are in the next event and JSD is ON
1295401372246 delta: 29ms
--- on activate ---

Then open:
http://getfirebug.com/tests/content/console/testErrors.html

Test 1: click on the button Syntax Error. 
Observed: error in the Error Console, nothing in the onError handler BUG

Test 2: click on the button Throw.
Observed: Error: uncaught exception: hi in the error console and the onError
handler, but no result from the onDebug handler BUG.

Comment 2

[John J. Barton]

Honza, please confirm you can run this.

Comment 3

[John J. Barton]

Cross reference:
http://code.google.com/p/fbug/issues/detail?id=3929

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Test extension

I can verify, the test extension can be used to reproduce the bug (using STR in comment #1)

I am also attaching XPI of the extension so, it's easier to install.
Honza

Comment 5

[Johnathan Nightingale [:johnath]]

Rob/Jim - feels like this should be hardblocker, but I yield to you on the triage there.

Comment 6

[John J. Barton]

Ok, my bad: there are two bug reports here, and now that I think about it, they are probably completely different:
  1. onError not getting syntax errors, probably something in jaegermonkey integration,
  2. onDebug not getting triggered, probably entirely inside of jsd.

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

syntax errors happen before JM is involved, so you should be able to reproduce it with all JITs turned off -- can you?

Also, when you say "no longer", do you know when it broke?

Comment 8

[Kevin Dangoor]

I spoke with Honza this morning and he's going to get a bit closer on the regression time. He said it was within the "last couple of days".

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

It's actually more than 2 days, but the range is following:

jsd.onError *is* executed (2011-01-07-13-mozilla-central/)
http://hg.mozilla.org/mozilla-central/rev/f93c5678d642

jsd.onError is *not* executed (2011-01-08-03-mozilla-central/)
http://hg.mozilla.org/mozilla-central/rev/8f1c39add00f

So, something happen between those two nightly builds.

Honza

Comment 10

[Boris Zbarsky [:bzbarsky]]

Please don't clobber blocking flags; you really do want this to block.  ;)

Can you find a TM regression range?  The range above is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f93c5678d642&tochange=8f1c39add00f and my money is on the TM merge.

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

According to my testing the jsd.onError breaks here:

TM: http://hg.mozilla.org/tracemonkey/rev/05cbcfe5d694 - OK
TM: http://hg.mozilla.org/tracemonkey/rev/fed63a7f54aa - ERROR

2011-01-07 -> 2011-01-08

Honza

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Now I noticed that I removed the blocking flag (again) by posting the message, but it was not intentional (I didn't touch the field!) and also I can't change it back...

Honza

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Sorry, for spamming this thread, but it looks like the problem with the flag was that I had this bug-page opened all the time in the browser and bugzilla didn't detect the "mid-air collision" when I submitted my comments. That's probably why the flag was overwritten. I have reported a new bug to track this issue:
https://bugzilla.mozilla.org/show_bug.cgi?id=627145

Honza

Comment 14

[Boris Zbarsky [:bzbarsky]]

That's http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=05cbcfe5d694&tochange=fed63a7f54aa

I'll bet this is a regression from bug 621845, given the range.

Comment 15

[Andreas Gal :gal]

I can reproduce an assert with the xpi. Working on that. Honza, thanks a ton for attaching it. Once I get past that point I will debug the underlying failure.

Comment 16

[Andreas Gal :gal]

the patch in bug 627943 fixes this for me, can you guys verify?

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to comment #16)
> the patch in bug 627943 fixes this for me, can you guys verify?
I applied the patch on tracemonkey branch, but it didn't fix it for me. I am still seeing the same behavior as described in the comment #1
Honza

Comment 18

[Andreas Gal :gal]

Alright, I will have to reproduce harder then. I got a crash in debug build, that is gone for me with the fix referenced above.

Comment 19

[Steve Fink [:sfink] [:s:]]

Works for me with current tracemonkey (d461afeeae3d). Let me verify my testing, though. I performed the following steps:

1. Installed the XPI attached to this bug
2. Navigated to http://getfirebug.com/tests/content/console/testErrors.html
3. Clicked on the buttons "SyntaxError" and "Throw". On stdout, I see

jsd.errorHook called SyntaxError: identifier starts immediately after numeric literal
JavaScript error: http://getfirebug.com/tests/content/console/testErrors.html, line 1: identifier starts immediately after numeric literal
WARNING: NS_ENSURE_SUCCESS(result, result) failed with result 0x80070057: file /home/sfink/src/TM-singlestep/content/events/src/nsEventListenerManager.cpp, line 1065
jsd.errorHook called uncaught exception: hi
JavaScript error: , line 0: uncaught exception: hi

Ignoring the NS_ENSURE_SUCCESS failure and the zero line number, it looks like the errors are being seen.

If you can still reproduce, can you attach a log of stdout (I guess that's what you called the "os console" on windows) for a failed run?

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I have been testing with tracemonkey build from:
http://hg.mozilla.org/tracemonkey/rev/9b510ceacb9d

After clicking on the buttons "SyntaxError" and "Throw". On stdout, I see:

jsd.errorHook called SyntaxError: identifier starts immediately after numeric literal
jsd.errorHook called uncaught exception: hi

This is a progress since it means that jsd.onError is properly called.
But jsd.onDebug is not and it should be since jsd.onError returns false.

Honza

Comment 21

[Steve Fink [:sfink] [:s:]]

Ok, finally got a chance to dig into this.

This sometimes works -- both onError and onDebug are called part of the time. But not all the time, and the attached extension does indeed show the problem.

What seems to be happening is that an uncaught exception is thrown, then handled in LAST_FRAME_CHECKS with an empty stack. That does the JSD callback, but when it calls jsd_NewThreadState() it sees an empty stack and gives up. (This is all for an "onclick" handler.)

In this test, onError works sometimes when onDebug doesn't, but that's because its callback goes straight into a SharedStub() thingie without going through jsd_CallExecutionHook (which is what does jsd_NewThreadState).

I'm not sure why this changed, though. I will continue digging.

Comment 22

[Steve Fink [:sfink] [:s:]]

(In reply to comment #21)
> I'm not sure why this changed, though. I will continue digging.

Ah. As bz pointed out, bug 621845 is very definitely related to this.

Comment 23

[Rob Campbell [:rc] (:robcee)]

Looks like Honza narrowed the regression range in Comment #11. Bz isolated that further in Comment #14 and Steve verified that bug 621845 is related. Removing regressionwindow-wanted from keywords.

Comment 24

[Steve Fink [:sfink] [:s:]]

These really are two separate issues. Honza's regression range was for the onError problem, not onDebug.

I can't find a revision where onDebug works for this test case. Does anyone know of one, no matter how old? Using 05cbcfe5d694 I do not see onDebug getting called (for the same reason; the stack is empty.)

I don't understand why this would ever have worked for uncaught exceptions.

Comment 25

[John J. Barton]

I can tell you from practical experience that onDebug is called sometimes. And I can tell you I believe it fails sometimes:
Bug 465672 - [jsd] jsdIDebuggerService onError or onDebug is broken

Comment 26

[John J. Barton]

Because of the API here we cannot help you with the onDebug part until you land the onError part.

Comment 27

[Steve Fink [:sfink] [:s:]]

(In reply to comment #25)
> I can tell you from practical experience that onDebug is called sometimes. And
> I can tell you I believe it fails sometimes:
> Bug 465672 - [jsd] jsdIDebuggerService onError or onDebug is broken

Yes, thank you! That bug describes exactly what I am observing. I should have searched better; I would have saved myself a lot of time.

It appears that this is not a regression. It's a missing feature (or the current state is a misfeature, perhaps.)

(In reply to comment #26)
> Because of the API here we cannot help you with the onDebug part until you land
> the onError part.

I believe this is already fixed. See comment 20.

It appears to me that this bug should be marked resolved (FIXEDBYSOMETHINGELSE), and bug 465672 either reworded or cloned into a feature request for better handling of uncaught exceptions.

Unless you have a test case that shows that onError is still broken, or onDebug is broken in more cases than it was previously?

Comment 28

[Rob Campbell [:rc] (:robcee)]

That's great news! (I think) :)

John, are you still seeing problems with onError handling or can we mark this bug fixed?

It would still be nice to know what fixed the original problem with onError, but I guess we have a rough timeframe based on Honza's comment.

Comment 29

[John J. Barton]

We can let you know once a build is available that does not crash.

Comment 30

[Steve Fink [:sfink] [:s:]]

(In reply to comment #29)
> We can let you know once a build is available that does not crash.

You have high standards, don't you? :-)

http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-6f223bba66ab/

has all of my changes to date, but that still has the methodjit crash. It sounds like a fix for that may be imminent. I will send you a link once it is available.

Comment 31

[LegNeato]

According to comment 27 (and comment 20) the onError stuff is fixed by something out. onDebug never worked and is tracked by bug 465672. The blocking part was onError, so this bug should be closed, correct?

Comment 32

[LegNeato]

/something out/something else/

Comment 33

[David Mandelin [:dmandelin]]

I'm told that:

- onError is working.

- onDebug is not a regression from 3.6.

So marking WFM. I think this is too late in the release cycle to consider beefing up onDebug functionality. But I do want to get whatever feature it is intended to enable working soon (e.g., in a release 3 months after Fx4). I also want to have a good design discussion on that, so we can add engine features if necessary to make it easier and more robust for Firebug to use.

Comment 34

[Steve Fink [:sfink] [:s:]]

I talked to Honza, and we found cases where onDebug *has* regressed. This bug has too much unrelated stuff, so I've opened bug 634434 which should inherit the hardblocking status.