From c814a01217085ebf21be46b1bcf06863099018af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 6 May 2020 12:45:14 +0200 Subject: [PATCH] Fix #42: Hack around React's development mode error triggering hack. React's development mode has a funny way of triggering errors, which tries to convince that exceptions are uncaught (for their development tools to report them) even though React catches them for the "error boundaries" feature. Since our jsdom handler reports uncaught exceptions as hard failures that fail the run, this creates a very bad interaction where every caught-but-not-really exception crashes the run. We hack around this hack by detecting when our error handler is in fact called by React's own hack. In that case, we ignore the uncaught exception, and proceed with normal execution. We add a test that actually uses React's error boundaries, and makes sure that the React component can still detect the error and its error message. --- build.sbt | 10 +- .../jsenv/jsdomnodejs/JSDOMNodeJSEnv.scala | 9 ++ .../jsdomnodejs/JSDOMNodeJSEnvTest.scala | 102 ++++++++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index afd9dd9..af16efd 100644 --- a/build.sbt +++ b/build.sbt @@ -75,7 +75,15 @@ lazy val `scalajs-env-jsdom-nodejs`: Project = project.in(file("jsdom-nodejs-env "org.scala-js" %% "scalajs-env-nodejs" % scalaJSVersion, "com.novocode" % "junit-interface" % "0.11" % "test", - "org.scala-js" %% "scalajs-js-envs-test-kit" % scalaJSVersion % "test" + "org.scala-js" %% "scalajs-js-envs-test-kit" % scalaJSVersion % "test", + + /* See JSDOMNodeJSEnvTest.reactUnhandledExceptionHack. + * We use intransitive() because we do not need the transitive + * dependencies of these webjars, and one of them actually fails to + * resolve (see https://github.com/webjars/webjars/issues/1789). + */ + "org.webjars.npm" % "react" % "16.13.1" % "test" intransitive(), + "org.webjars.npm" % "react-dom" % "16.13.1" % "test" intransitive(), ) ) diff --git a/jsdom-nodejs-env/src/main/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnv.scala b/jsdom-nodejs-env/src/main/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnv.scala index 0ef7777..9193c2d 100644 --- a/jsdom-nodejs-env/src/main/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnv.scala +++ b/jsdom-nodejs-env/src/main/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnv.scala @@ -84,6 +84,15 @@ class JSDOMNodeJSEnv(config: JSDOMNodeJSEnv.Config) extends JSEnv { | var virtualConsole = new jsdom.VirtualConsole() | .sendTo(console, { omitJSDOMErrors: true }); | virtualConsole.on("jsdomError", function (error) { + | /* #42 Counter-hack the hack that React's development mode uses + | * to bypass browsers' debugging tools. If we detect that we are + | * called from that hack, we do nothing. + | */ + | var isWithinReactsInvokeGuardedCallbackDevHack_issue42 = + | new Error("").stack.indexOf("invokeGuardedCallbackDev") >= 0; + | if (isWithinReactsInvokeGuardedCallbackDevHack_issue42) + | return; + | | try { | // Display as much info about the error as possible | if (error.detail && error.detail.stack) { diff --git a/jsdom-nodejs-env/src/test/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnvTest.scala b/jsdom-nodejs-env/src/test/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnvTest.scala index 4199f9d..822ae1d 100644 --- a/jsdom-nodejs-env/src/test/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnvTest.scala +++ b/jsdom-nodejs-env/src/test/scala/org/scalajs/jsenv/jsdomnodejs/JSDOMNodeJSEnvTest.scala @@ -1,12 +1,20 @@ package org.scalajs.jsenv.jsdomnodejs +import java.nio.charset.StandardCharsets +import java.nio.file.{Files, Path} + import scala.concurrent.duration._ +import com.google.common.jimfs.Jimfs + import org.junit.Test +import org.scalajs.jsenv.Input import org.scalajs.jsenv.test.kit.TestKit class JSDOMNodeJSEnvTest { + import JSDOMNodeJSEnvTest._ + private val kit = new TestKit(new JSDOMNodeJSEnv, 1.minute) @Test @@ -21,4 +29,98 @@ class JSDOMNodeJSEnvTest { .expectOut("http://localhost/foo\n") } } + + @Test + def reactUnhandledExceptionHack_issue42: Unit = { + val code = + """ + |const rootElement = document.createElement("div"); + |document.body.appendChild(rootElement); + | + |class ThrowingComponent extends React.Component { + | render() { + | throw new Error("boom"); + | } + |} + | + |class ErrorBoundary extends React.Component { + | constructor(props) { + | super(props); + | this.state = { hasError: false }; + | } + | + | componentDidCatch(error, info) { + | this.setState({error: error.message, hasError: true}); + | } + | + | render() { + | if (this.state.hasError) { + | console.log("render-error"); + | return React.createElement("p", null, + | `Caught error: ${this.state.error}`); + | } else { + | return this.props.children; + | } + | } + |} + | + |class MyMainComponent extends React.Component { + | render() { + | console.log("two"); + | return React.createElement(ErrorBoundary, null, + | React.createElement(ThrowingComponent) + | ); + | } + |} + | + |console.log("begin"); + | + |const mounted = ReactDOM.render( + | React.createElement(ErrorBoundary, null, + | React.createElement(ThrowingComponent, null) + | ), + | rootElement + |); + | + |console.log(document.querySelector("p").textContent); + | + |console.log("end"); + """.stripMargin + + kit.withRun(ReactJSFiles :+ codeToInput(code)) { + _.expectOut("begin\nrender-error\nCaught error: boom\nend\n") + .succeeds() + } + } +} + +object JSDOMNodeJSEnvTest { + private lazy val ReactJSFiles: List[Input] = { + val fs = Jimfs.newFileSystem() + val reactFile = copyResource( + "/META-INF/resources/webjars/react/16.13.1/umd/react.development.js", + fs.getPath("react.development.js")) + val reactDOMFile = copyResource( + "/META-INF/resources/webjars/react-dom/16.13.1/umd/react-dom.development.js", + fs.getPath("react-dom.development.js")) + List(reactFile, reactDOMFile).map(Input.Script(_)) + } + + private def copyResource(name: String, out: Path): out.type = { + val inputStream = getClass().getResourceAsStream(name) + assert(inputStream != null, s"couldn't load $name from resources") + try { + Files.copy(inputStream, out) + } finally { + inputStream.close() + } + out + } + + private def codeToInput(code: String): Input = { + val p = Files.write( + Jimfs.newFileSystem().getPath("testScript.js"), + code.getBytes(StandardCharsets.UTF_8)) + Input.Script(p) + } }