#214 Provide an overload for JavaScriptEngine.callFunction that can always throw exception

[Goktug]

callFunction methods in JavaScriptEngine may throw an exception or not based on throwExceptionOnScriptError option provided to WebClient.

This is a problem with GWT because we can really mimic the browser behavior. If the throwExceptionOnScriptError is false, htmlunit works like browser and continues working as exceptions appear. However GWT also relies on JSE.callFunction for development mode which swallows the exception if the option is set to false; which would not be the case in a browser.

If we choose to set the option true for web mode, this time it causes other problems. For example, the caller of btn.click will receive any exception thrown from the handler unlike what happens in a real browser. So it looks like we really need to set throwExceptionOnScriptError to false even in dev-mode.

The simplest solution looks like providing an alternative for 'callFunction' that will throw exception independent of the option provided to WebClient.

[Marc Guillemot]

I still don't understand the issue. If you want HtmlUnit to behave like "real" browsers, you have to set throwExceptionOnScriptError to false. In this case, if you need the exception, you have to use a JavaScriptErrorListener.

[Goktug]

Let me try to explain in a different way.

GWT has two modes.

One is script mode (a.k.a. production mode) where we output javascript code from java that can be executed as a regular js in browser. Thinking this part alone, setting throwExceptionOnScriptError to false makes perfect sense and everything will work as expected. No problems here that I'm aware of.

The second mode is called dev mode (a.k.a hosted mode). This mode is pretty complicated but in very basic terms, GWT launches a java process that executes GWT code and interfaces with javascript and DOM via a plugin inside the browser. This plugin is responsible for executing any javascript snippet requested by the GWT java process. To do that, GWT injects a function to page and plugin calls this function to evaluate the request javascript from the java process.

To emulate this in htmlunit GWT uses the JavaScriptEngine.callFunction method and expects it to throw exception if the underlying code throws exception so that it can translate and send it back to java process.
This is one direction of communication; there is also the other direction. Javascript code on the browser may call the java code so the plugin needs to intercept and forward those calls to java process.

All of this is a synchronous re-entrant operation. So java can call javascript, javascript can call java, and so on.
I'll try to illustrate an example call stack:

java process -> JavascriptEngine.callFunction -> some_js_code -> GWT's ScriptableObject -> java process -> JavascriptEngine.callFunction .....

As you can see, there are multiple on going calls, it would be likely possible but would be much more complex and error prone to use a central exception listener and try to associate exceptions with specific calls.

When a real browser plugin calls a function inside the page, the plugin will receive the exception as a return value or via catch block. With current APIs in htmlunit, I'm not aware of anyway to easily mimic that and that is why I suggested this change. From an outsider's perspective, it makes sense to me let JavaScriptEngine to be able to throw exceptions independent from the options that are set in webclient. Easiest and least impactful change I found to achieve this is to provide an overload to callFunction.

Please let me know if you have other suggestions.

[Marc Guillemot]

I'm aware of GWT's modes. Thanks.

Could you provide the unit test(s) illustrating what you are looking for? This would ensure that we're talking about the same thing and this is absolutely necessary for a new feature in HtmlUnit.

[Goktug]

I don't have much familiarity with htmlunit so I wrote it looking at GWT usage and some tests inside htmlunit. I don't have a htmlunit development environment setup so this may not be even compiling but here it is:

@Test
void callFunctionThrowsException() {
  String content = "<html><head><title>foo</title><script>\n"
    + "window.throwsSomeException = function(){ doesntExists.call(); };"
    + "</script></head><body></body></html>";

  HtmlPage page = loadPage(content);

  Window window = (Window) page.getEnclosingWindow().getScriptObject();
  Function jsFunction = (Function) ScriptableObject.getProperty(
        window, "throwsSomeException");
  JavaScriptEngine jse = getWebClient().getJavaScriptEngine();

  try {
    jse.callFunction(page, jsFunction, window, window, new Object[0]);
    // This would be a breaking change because callFunction doesn't do the 
    // the reporting of errors via listener; instead caller does.
    // This may require larger refactoring on htmlunit  so that's why you
    // want to overload instead:
    // or jse.callFunction(<.....>, true /* throw on exception*/);
    // or jse.callFunction2(<.....>);
    // or something similar
    fail("Should have failed");
  }
  catch (ScriptException se) {
    assertTrue(se.getCause() instanceof JavaScriptException);
  }
}

[Goktug]

Also, this is the supplemental test case that we need to be sure not broken:

@Test
@Alerts("foo")
void browserCallbackDoesntLeakExceptions() {
  String content = "<html><head><title>foo</title><script>\n"
    + "function onload() {\n"
    + "  var btn = document.getElementById('button1');"
    + "  btn.onClick = function() { doesntExists.call(); };"
    + "  btn.click();"
    + "  alert('foo');"
    + "}";
    + "</script></head><body>\n"
    + "<div id='button1' />"
    + "</body></html>";

  loadPageWithAlerts(content);
}

[Marc Guillemot]

Concerning the first test:

it fails on

assertTrue(se.getCause() instanceof JavaScriptException);

because the exception's cause is an EcmaError and not a JavaScriptException. This seems correct for me if you refer to the JavaDoc of JavaScriptException >Instances of this class are thrown by the JavaScript 'throw' keyword"<.

If you change for instance

    doesntExists.call();

to

    throw "boom"

then the test is green.

[Marc Guillemot]

I guess that you meant using

webClient.getOptions().setThrowExceptionOnScriptError(false);

in the second test before loading the HTML code.

Interestingly, this test has been showing that method click() is supported in real FF on any element at least since version 10 (but not in 3.6) what is not yet the case in HtmlUnit. I'm working on a fix.

[Goktug]

Yes, in second test case should be configured as

webClient.getOptions().setThrowExceptionOnScriptError(false);

And given this configuration I need a way to make the first test case pass.

Thanks for looking into it.

[Goktug]

Hi, Is there any progress on this one?