Re: [gwt-contrib] Re: Jetty upgrade broke HtmlUnit for window.onerror

2017-10-07 Thread Thomas Broyer


On Saturday, October 7, 2017 at 5:20:15 PM UTC+2, Thomas Broyer wrote:
>
>
>
> On Saturday, October 7, 2017 at 4:54:28 PM UTC+2, Colin Alworth wrote:
>>
>> Like we do for 
>> com.google.gwt.junit.RunStyleHtmlUnit.HostedJavaScriptEngine so we can hook 
>> in the "plugin". Looks like that idea might be a winner! Just make sure to 
>> swap it in both cases, don't want to kill tests in old dev mode.
>>
>
> Seems to be working. I still have 2 tests failing in JUnitSuite 
> (GWTTestCaseSetupTearDownTest#testSetUpTearDownTimeout and 
> GWTTestCaseAsyncTest#testLateFinsh_assert), both of them expecting 
> TimeoutException and getting a SerializableThrowable instead. Need to debug 
> this using -Dgwt.htmlunit.debug now (exception thrown from Impl.java:215, 
> i.e. Impl#reportToBrowser, the object is logged as "[object Object]" so I 
> need a debugger to understand what happens)
>

OK, so, there's a problem of isolation: the exception from 
testSetUpTearDownFailAsync is making testSetUpTearDownTimeout fail.
This is likely because GWTTestCase's reportUncaughtException (called from 
the Impl.uncaughtExceptionHandlerForTest) immediately 
reportResultsAndRunNextMethod, which schedules the next test with 
Scheduler.get().scheduleDeferred. Then (from Impl's 
reportUncaughtException), reportToBrowser is called which throws the 
exception again in a setTimeout(…, 0). scheduleDeferred uses setTimeout(…, 
1), but because HtmlUnit clamps the delay at 1ms anyway, the 
scheduleDeferred command is run before the exception is thrown to the 
browser.
Removing the setTimeout from Impl's reportToBrowser fixes the test, but 
(probably) only because reportUncaughtException is called by the top-most 
$entry(); otherwise it'd (probably) be caught by the parent $entry and 
re-routed to the UncaughtExceptionHandler.

Daniel, is there any other change you have internally that's not sync'd to 
open source? I don't quite understand how that could work even with the 
earlier HtmlUnit.

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/49111dbf-d8cc-47cc-983c-c9d13fe8301d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [gwt-contrib] Re: Jetty upgrade broke HtmlUnit for window.onerror

2017-10-07 Thread Thomas Broyer


On Saturday, October 7, 2017 at 4:54:28 PM UTC+2, Colin Alworth wrote:
>
> Like we do for 
> com.google.gwt.junit.RunStyleHtmlUnit.HostedJavaScriptEngine so we can hook 
> in the "plugin". Looks like that idea might be a winner! Just make sure to 
> swap it in both cases, don't want to kill tests in old dev mode.
>

Seems to be working. I still have 2 tests failing in JUnitSuite 
(GWTTestCaseSetupTearDownTest#testSetUpTearDownTimeout and 
GWTTestCaseAsyncTest#testLateFinsh_assert), both of them expecting 
TimeoutException and getting a SerializableThrowable instead. Need to debug 
this using -Dgwt.htmlunit.debug now (exception thrown from Impl.java:215, 
i.e. Impl#reportToBrowser, the object is logged as "[object Object]" so I 
need a debugger to understand what happens)

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/fcabaaa5-d15b-4452-9cbd-9f4c0e5ccee2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [gwt-contrib] Re: Jetty upgrade broke HtmlUnit for window.onerror

2017-10-07 Thread Colin Alworth
Like we do for
com.google.gwt.junit.RunStyleHtmlUnit.HostedJavaScriptEngine so we can
hook in the "plugin". Looks like that idea might be a winner! Just make
sure to swap it in both cases, don't want to kill tests in old dev mode.

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/1507388064.2777465.1131047224.1BEFE507%40webmail.messagingengine.com.
For more options, visit https://groups.google.com/d/optout.


[gwt-contrib] Re: Jetty upgrade broke HtmlUnit for window.onerror

2017-10-07 Thread Thomas Broyer


On Saturday, October 7, 2017 at 3:01:52 PM UTC+2, Colin Alworth wrote:
>
> Exactly - I wasn't planning on adding the javaToJs(), but was going to 
> unwrap the exception before calling onerror (or have ScriptException 
> implement Scriptable). Have a short test that demonstrates the issue 
> without gwt (but wow they have a lot of GWT in their source tree), and am 
> was waiting for my svn->git sync to finish to confirm what you did by hand. 
> Just in case we decide to rebase it along, or make other improvements down 
> the road...
>

I wonder if we couldn't workaround the issue with a custom 
JavaScriptEngine's handleJavaScriptException? (override it with the same 
code except with the triggerOnError call "inlined" with the "fix")
This possibly would allow us to have Daniel's change in 2.8.2, without the 
need to update or locally-patch HtmlUnit at all (and then wait for the 
fixed HtmlUnit before updating it, for 2.9, removing support for JDK 7 
entirely)

I'll dig this up while you work on a real fix ;-)

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/19c0880f-00b1-46b4-aaae-15b3fd79c978%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[gwt-contrib] Re: Jetty upgrade broke HtmlUnit for window.onerror

2017-10-07 Thread Colin Alworth
Exactly - I wasn't planning on adding the javaToJs(), but was going to 
unwrap the exception before calling onerror (or have ScriptException 
implement Scriptable). Have a short test that demonstrates the issue 
without gwt (but wow they have a lot of GWT in their source tree), and am 
was waiting for my svn->git sync to finish to confirm what you did by hand. 
Just in case we decide to rebase it along, or make other improvements down 
the road...

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/46ee5450-a14f-4a0f-a4c9-302d5a4c1709%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[gwt-contrib] Re: Jetty upgrade broke HtmlUnit for window.onerror

2017-10-07 Thread Thomas Broyer


On Friday, October 6, 2017 at 7:55:15 PM UTC+2, Colin Alworth wrote:
>
> Okay, I'm about 80% sure that I understand and can remedy the problem 
> within HtmlUnit itself. Will update once I finish syncing the apparently 
> canonical SVN repo to git, so I can go over the history more carefully and 
> ensure that this break isn't deliberate.
>

Ah, I think I understand too!
In 2.13 (before we updated), only the line number was passed to 
onerror: 
https://sourceforge.net/p/htmlunit/code/HEAD/tree/tags/HtmlUnit-2.13/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java#l1238
In 2.14, they started passing the column 
number: 
https://sourceforge.net/p/htmlunit/code/HEAD/tree/tags/HtmlUnit-2.14/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java#l1253
In 2.16, they started passing the 
exception: 
https://sourceforge.net/p/htmlunit/code/HEAD/tree/tags/HtmlUnit-2.16/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java#l1284
But note that this is the ScriptException, not a JS exception. This is 
possibly where Rhino's Context.javaToJS() is missing, as warned in the 
logs; or more accurately, the JS exception should probably be extracted out 
of the ScriptException (if (e.getCause() instanceof JavaScriptException) { 
args[4] = ((JavaScriptException) e).getValue(); }, 
see 
https://sourceforge.net/p/htmlunit/code/HEAD/tree/tags/HtmlUnit-2.19/src/main/java/com/gargoylesoftware/htmlunit/ScriptException.java#l138
 
for inspiration)
I traced that last change to https://sourceforge.net/p/htmlunit/code/9587/, 
made without a test…

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/dfac141c-c0fc-4649-bffb-a65dda025408%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.