The jsdom Node.js env intercepts a JS exception

[japgolly]

A bit of background first: React allows users to define a componentDidCatch lifecycle hook that is invoked when a child component throws an exception. Users can use this to maybe display the error message on the screen and prevent the error bubbling up further.

I have a test in scalajs-react that covers this behaviour. Since upgrading to Scala.JS 1.0, the error isn't caught by React. Instead it acts as if I typed in ??? rather than render.

I've confirmed that if I run the code in the browser, React catches and handles the error gracefully, rendering exactly what my test case expects. Therefore I think it's something about either the test runner or the node environment.

(Reproduction can be provided soon / on demand)

[japgolly]

I've also tried this with both jsdom 9.12.0, 15.something and 16.latest. Same results across the board. This is probably the same as scala-js/scala-js#3458

[sjrd]

A reproduction would be awesome!

Does this happen only with the jsdom environment, or can it be reproduce with another environment with DOM support like Selenium?

Harder question: can it be reproduced in a non-DOM environment?

[japgolly]

An isolated, minimised reproduction is basically too hard because it's in the React internals that the error is thrown and caught. But I can provide a reproduction using scalajs-react:

1. Checkout https://github.com/japgolly/scalajs-react/tree/tmp/4029
2. sbt
3. test/testOnly -- japgolly.scalajs.react.core.ScalaComponentPTest.lifecycle1

Error occurs at https://github.com/japgolly/scalajs-react/blob/tmp/4029/test/src/test/scala/japgolly/scalajs/react/core/ScalaComponentTest.scala#L171

Regarding the different envs:

• JSDOMNodeJSEnv - problem occurs as described
• SeleniumJSEnv + Chrome - no problems
• SeleniumJSEnv + Firefox - no problems (except the exception provided in the componentDidCatch hook has a different message: "next$1 is null" instead of Cannot read property of null)
• no DOM - unfortunately the only way I know to reproduce this is via ReactDOM which requires dom

[japgolly]

Also try ... catch doesn't catch the error either 😨

[sjrd]

When I try your repro, I hit the FileSystemLoopException exception from the other report. What did you change to get to running the test to reproduce the above?

[japgolly]

Try with JDK 11

[sjrd]

Try with JDK 11

Thanks. Now I can reproduce this.

Just FTR, one's got to love a cs-based installation of Scala. Even on Windows, trying this was as easy as

$ cs launch --jvm 11 sbt-launcher

[sjrd]

FWIW, I can make the scalajs-react test pass if I comment out/delete the following lines:

scala-js-env-jsdom-nodejs/jsdom-nodejs-env/src/main/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnv.scala

Lines 88 to 102 in e2adf2b

	| .sendTo(console, { omitJSDOMErrors: true });
	| virtualConsole.on("jsdomError", function (error) {
	| try {
	| // Display as much info about the error as possible
	| if (error.detail && error.detail.stack) {
	| console.error("" + error.detail);
	| console.error(error.detail.stack);
	| } else {
	| console.error(error);
	| }
	| } finally {
	| // Whatever happens, kill the process so that the run fails
	| process.exit(1);
	| }
	| });

However, if I do that, all the tests that make sure that a truly uncaught exception causes the invocation to fail start to fail (i.e., an uncaught exception does not fail the run, whereas it should).

[sjrd]

I can also make the test pass if I keep the setup, but I empty the content of the function (error) { ... }. So my current strategy is to, in that function, somehow detect that we're being invoked from React's hack, to counter-hack it and just return, without displaying the error and exit the process.

[gzm0]

What is cs?

[sjrd]

What is cs?

The coursier CLI, that we're hoping to make the default way to install Scala.

[japgolly]

It's curious that the Selenium jsenv doesn't have this issue. Presumably it's also got some code for catching errant exceptions yet doesn't catch this one that React does.

[sjrd]

Selenium fails a run by default if there is an uncaught exception. We don't have to convince it to do so.

I don't understand why React's code ends up as a "jsdomError". AFACIT it should trigger an "error", not "jsdomError". This is why things go wrong in jsdom I think.

[sjrd]

Hum ... rereading https://github.com/jsdom/jsdom#virtual-consoles, apparently "jsdomError" is intentionally triggered when there is an uncaught exception not handled by a window onerror handler. They say:

This is similar to how error messages often show up in web browser consoles, even if they are not initiated by console.error.

And React's hack is precisely going out of its way to cause browsers to think it's an uncaught error, even though it is actually "caugh". See the big comment at https://github.com/facebook/react/blob/3e94bce765d355d74f6a60feb4addb6d196e3482/packages/shared/invokeGuardedCallbackImpl.js#L32

So that explains why a "jsdomError" is reported. It's a combination of the intentional behavior of both jsdom and React's hack.

[sjrd]

@japgolly I managed to minimize the lifecycle1 test case to the following, which still exhibits the bug, and still passes with my counter-hack:

    "lifecycle1" - {
      class Props(val a: Int)

      val Inner = ScalaComponent.builder[Props]("")
        .stateless
        .render_P(p => raw.React.createElement("div", null, p.a.toString()))
        .build

      val Comp = ScalaComponent.builder[Props]("")
        .initialState[Option[String]](None) // error message
        .render_PS((p, s) => s match {
          case None    => Inner(p).vdomElement
          case Some(e) => raw.React.createElement("div", null, "Error: " + e)
        })
        .componentDidCatch($ => $.setState(Some($.error.message.replaceFirst("'.+' *", ""))))
        .build

      val staleDomNodeCallback = ReactTestUtils.withNewBodyElement { mountNode =>
        val comp = Comp(null)
        println("OK HERE WE GO.........")
        val mounted = comp.renderIntoDOM(mountNode) // <---------------------------- TROUBLE
        println("WOW IT WORKED!")

        mounted.withEffectsPure.getDOMNode
      }

      staleDomNodeCallback.runNow()
    }

Would you be able to give me a JavaScript-only version of the above logic? Something that doesn't use Scala.js at all (and hence not scalajs-react); just plain React.js.

That would be a test case I would be able to include in the test suite of scalajs-env-jsdom-nodejs, for a PR that fixes this issue.

[sjrd]

OK, I've got a reproducing test case, and a hack that fixes it. Just leaving it here FTR; I'll send a PR tomorrow:

  @Test
  def reactUnhandledExceptionHack: Unit = {
    val code =
      """
      |var rootElement = document.createElement("div");
      |document.body.appendChild(rootElement);
      |
      |class BasicComponent extends React.Component {
      |  render() {
      |    throw new Error("boom");
      |    return React.createElement("p", null, "Content");
      |  }
      |}
      |
      |class ErrorBoundary extends React.Component {
      |  constructor(props) {
      |    super(props);
      |    this.state = { hasError: false };
      |  }
      |
      |  componentDidCatch(error, info) {
      |    // Display fallback UI
      |    this.setState({ hasError: true });
      |    // You can also log the error to an error reporting service
      |    //logErrorToMyService(error, info);
      |  }
      |
      |  render() {
      |    if (this.state.hasError) {
      |      // You can render any custom fallback UI
      |      //throw new Error("reboom");
      |      console.log("render-error");
      |      return React.createElement("h1", null, "Something went wrong");
      |    }
      |    return this.props.children;
      |  }
      |}
      |
      |class MyMainComponent extends React.Component {
      |  constructor(...args) {
      |    super(...args);
      |    console.log("constr");
      |  }
      |  render() {
      |    console.log("two");
      |    //throw new Error("main error");
      |    return React.createElement("div", null,
      |      React.createElement(ErrorBoundary, null,
      |        React.createElement(BasicComponent)
      |      )
      |    );
      |  }
      |}
      |
      |console.log("one");
      |
      |ReactDOM.render(
      |  React.createElement(MyMainComponent, null),
      |  rootElement
      |);
      |
      |setTimeout(function() { console.log("end"); }, 1000);
      """.stripMargin

    kit.withRun(ReactJSFiles :+ codeToInput(code)) {
      _.expectOut("one\nconstr\ntwo\nrender-error\nend\n")
        .closeRun()
    }
  }

[japgolly]

Sure:

class Inner extends React.Component {
  render() {
    let p = this.props;
    p = p.minus(p);
    return null;
  }
}

class Comp extends React.Component {
  constructor(props) {
    super(props);
    this.state = { hasError: false };
  }

  render() {
    if (this.state.hasError)
      return React.createElement("div", null, `Error: ${this.state.error}`)
    else
      return React.createElement(Inner, this.props)
  }

  componentDidCatch(e) {
    this.setState({error: e.message, hasError: true})
  }
}

const root = document.createElement('div');
const props = React.createElement(Comp, null)
const mounted = ReactDOM.render(props, root)

[japgolly]

Oh whoops, I should've read all replies before I replied. :)

[sjrd]

Oh whoops, I should've read all replies before I replied. :)

At least it confirms that I had understood the various pieces. 😅