jsd.onError and jsd.onDebug fail

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
RESOLVED WORKSFORME
6 years ago
6 years ago

People

(Reporter: John J. Barton, Assigned: sfink)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [firebug-p1][hardblocker])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
jsd.onError is no longer called for JavaScript syntax errors.

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

Testcase next.
blocking2.0: --- → ?
(Reporter)

Comment 1

6 years ago
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.
(Reporter)

Comment 2

6 years ago
Honza, please confirm you can run this.
Whiteboard: [firebug-p1]
(Reporter)

Comment 3

6 years ago
Cross reference:
http://code.google.com/p/fbug/issues/detail?id=3929
Created attachment 505002 [details]
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
Rob/Jim - feels like this should be hardblocker, but I yield to you on the triage there.
blocking2.0: ? → final+
Keywords: regressionwindow-wanted
(Reporter)

Comment 6

6 years ago
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.
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

6 years ago
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".
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
blocking2.0: final+ → ?
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.
blocking2.0: ? → final+
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
blocking2.0: final+ → ?
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
blocking2.0: ? → betaN+
Whiteboard: [firebug-p1] → [firebug-p1][hardblocker]
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
blocking2.0: betaN+ → ?

Updated

6 years ago
blocking2.0: ? → betaN+
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.

Updated

6 years ago
Assignee: nobody → gal

Comment 15

6 years ago
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

6 years ago
the patch in bug 627943 fixes this for me, can you guys verify?
(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

6 years ago
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.

Updated

6 years ago
Assignee: gal → sphink

Updated

6 years ago
(Assignee)

Comment 19

6 years ago
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?
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
(Assignee)

Comment 21

6 years ago
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.
(Assignee)

Comment 22

6 years ago
(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.
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.
Status: NEW → ASSIGNED
Keywords: regressionwindow-wanted → regression
(Assignee)

Comment 24

6 years ago
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.
(Reporter)

Comment 25

6 years ago
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
(Reporter)

Comment 26

6 years ago
Because of the API here we cannot help you with the onDebug part until you land the onError part.
(Assignee)

Comment 27

6 years ago
(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?
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.
Keywords: regression
(Reporter)

Comment 29

6 years ago
We can let you know once a build is available that does not crash.
(Assignee)

Comment 30

6 years ago
(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/sfink@mozilla.com-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

6 years ago
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

6 years ago
/something out/something else/
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
(Assignee)

Comment 34

6 years ago
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.
Component: JavaScript Debugging/Profiling APIs → JavaScript Engine
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.