Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions escape and crash the entire jsEnv #47

Open
japgolly opened this issue Jun 9, 2020 · 4 comments · May be fixed by #48
Open

Exceptions escape and crash the entire jsEnv #47

japgolly opened this issue Jun 9, 2020 · 4 comments · May be fixed by #48

Comments

@japgolly
Copy link

japgolly commented Jun 9, 2020

It's perfectly, completely and deterministically reproducible in my private code that the new SJS 1.0 PhantomJs jsEnv doesn't catch exceptions in some cases. I haven't been able to work out what "some cases" means exactly. I've got code that looks something like this:

      def x(f: => Any) = try Right(f) catch { case e: Throwable => Left(e) }
      x(???)

and instead of the error my test is expecting being caught, it seems to be intercepted by PhantomJs and causes this:

scala.MatchError: 2 (of class java.lang.Byte)

  phantomjs://code/phantomjs-launcher8807894677851635821.js:8 in onError


  phantomjs://code/phantomjs-launcher8807894677851635821.js:10 in onError
  file:///tmp/tmp-9624954527195675435webapp-base-test-test-fastopt.js:396685 (in function "$p_jl_JSConsoleBasedPrintStream__doWriteLine__T__V")

  phantomjs://code/phantomjs-launcher8807894677851635821.js:12
[error] stack trace is suppressed; run last Test / testOnly for the full output
[error] (Test / testOnly) org.scalajs.testing.common.RPCCore$ClosedException: org.scalajs.testing.adapter.JSEnvRPC$RunTerminatedException
[error] Total time: 9 s, completed 9 Jun. 2020, 9:03:20 pm

We actually went over this ages ago in scala-js/scala-js#1555 and I came away with a local hack that avoided the problem for me consistently for years, until now.

@japgolly japgolly linked a pull request Jun 9, 2020 that will close this issue
@japgolly
Copy link
Author

japgolly commented Jun 9, 2020

The fix is super easy so I've raised #48 .

@japgolly
Copy link
Author

japgolly commented Jun 9, 2020

For reference: the fix I used to have is:

class PhantomJS2Env(c: PhantomJSEnv.Config) extends PhantomJSEnv(c) {
 
  override protected def vmName: String = "PhantomJS2"
 
  private val consoleNuker = new MemVirtualJSFile("consoleNuker.js")
    .withContent("console.error = console.log;")
 
  override protected def customInitFiles(): Seq[VirtualJSFile] =
    super.customInitFiles() :+ consoleNuker
}

which doesn't work anymore because PhantomJSEnv is final (which is a good thing).

@gzm0
Copy link
Contributor

gzm0 commented Jun 9, 2020

The workaround could now be written as:

final class PhantomJS2Env(config: PhantomJSEnv.Config) extends JSEnv {

  private val innerEnv = new PhantomJSEnv(c)
  val name: String = "PhantomJS2"
 
  private val consoleNuker: Input = {
    val p = Files.write(
        Jimfs.newFileSystem().getPath("consoleNuker.js"),
        "console.error = console.log;".getBytes(StandardCharsets.UTF_8))
    Input.Script(p)
  }
 
  def start(input: Seq[Input], config: RunConfig): JSRun =
    innerEnv.start(consoleNuker :: input, config)

  def startWithCom(input: Seq[Input], config: RunConfig,
      onMessage: String => Unit): JSComRun =
    innerEnv.startWithCom(consoleNuker :: input, config, onMessage)
}

@japgolly
Copy link
Author

Oh wow thanks @gzm0 ! I didn't think of doing that. Actually I came up with my own hack too :)

project/phantomjs-fix.js

console.error = console.log;

and then

Test / jsEnvInput := Input.Script(((ThisBuild / baseDirectory).value / "project/phantomjs-fix.js").toPath) +: (Test / jsEnvInput).value)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants