Merge pull request #4661 from yglukhov/js-uncaught-stacktrace

Uncaught exceptions in JS now always propagate with better stack trace. Fixed codegen bug.

6 files changed, 71 insertions, 46 deletions

diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim
index 80bcd2b0e..3e56e2f41 100644
--- a/compiler/jsgen.nim
+++ b/compiler/jsgen.nim
@@ -512,6 +512,10 @@ proc arith(p: PProc, n: PNode, r: var TCompRes, op: TMagic) =
     arithAux(p, n, r, op, jsOps)
   r.kind = resExpr
 
+proc hasFrameInfo(p: PProc): bool =
+  ({optLineTrace, optStackTrace} * p.options == {optLineTrace, optStackTrace}) and
+      ((p.prc == nil) or not (sfPure in p.prc.flags))
+
 proc genLineDir(p: PProc, n: PNode) =
   let line = toLinenumber(n.info)
   if optLineDir in p.options:
@@ -521,9 +525,7 @@ proc genLineDir(p: PProc, n: PNode) =
       ((p.prc == nil) or sfPure notin p.prc.flags):
     useMagic(p, "endb")
     addf(p.body, "endb($1);$n", [rope(line)])
-  elif ({optLineTrace, optStackTrace} * p.options ==
-      {optLineTrace, optStackTrace}) and
-      ((p.prc == nil) or not (sfPure in p.prc.flags)):
+  elif hasFrameInfo(p):
     addf(p.body, "F.line = $1;$n" | "$$F['line'] = $1;$n", [rope(line)])
 
 proc genWhileStmt(p: PProc, n: PNode) =
@@ -558,10 +560,13 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) =
   # code to generate:
   #
   #  ++excHandler;
+  #  var tmpFramePtr = framePtr;
   #  try {
   #    stmts;
+  #    --excHandler;
   #  } catch (EXC) {
   #    var prevJSError = lastJSError; lastJSError = EXC;
+  #    framePtr = tmpFramePtr;
   #    --excHandler;
   #    if (e.typ && e.typ == NTI433 || e.typ == NTI2321) {
   #      stmts;
@@ -572,6 +577,7 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) =
   #    }
   #    lastJSError = prevJSError;
   #  } finally {
+  #    framePtr = tmpFramePtr;
   #    stmts;
   #  }
   genLineDir(p, n)
@@ -584,8 +590,10 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) =
   var catchBranchesExist = length > 1 and n.sons[i].kind == nkExceptBranch
   if catchBranchesExist and p.target == targetJS:
     add(p.body, "++excHandler;" & tnl)
-  var safePoint = "Tmp$1" % [rope(p.unique)]
-  if optStackTrace in p.options: add(p.body, "framePtr = F;" & tnl)
+  var tmpFramePtr = rope"F"
+  if optStackTrace notin p.options:
+    tmpFramePtr = p.getTemp(true)
+    add(p.body, tmpFramePtr & " = framePtr;" & tnl)
   addf(p.body, "try {$n", [])
   if p.target == targetPHP and p.globals == nil:
       p.globals = "global $lastJSError; global $prevJSError;".rope
@@ -595,8 +603,9 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) =
   var generalCatchBranchExists = false
   let dollar = rope(if p.target == targetJS: "" else: "$")
   if p.target == targetJS and catchBranchesExist:
-    addf(p.body, "} catch (EXC) {$n var prevJSError = lastJSError;$n" &
+    addf(p.body, "--excHandler;$n} catch (EXC) {$n var prevJSError = lastJSError;$n" &
         " lastJSError = EXC;$n --excHandler;$n", [])
+    add(p.body, "framePtr = $1;$n" % [tmpFramePtr])
   elif p.target == targetPHP:
     addf(p.body, "} catch (Exception $$EXC) {$n $$prevJSError = $$lastJSError;$n $$lastJSError = $$EXC;$n", [])
   while i < length and n.sons[i].kind == nkExceptBranch:
@@ -618,8 +627,7 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) =
         addf(orExpr, "isObj($2lastJSError.m_type, $1)",
              [genTypeInfo(p, n.sons[i].sons[j].typ), dollar])
       if i > 1: add(p.body, "else ")
-      addf(p.body, "if ($3lastJSError && ($2)) {$n",
-        [safePoint, orExpr, dollar])
+      addf(p.body, "if ($1lastJSError && ($2)) {$n", [dollar, orExpr])
       gen(p, n.sons[i].sons[blen - 1], a)
       moveInto(p, a, r)
       addf(p.body, "}$n", [])
@@ -631,6 +639,7 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) =
     addf(p.body, "$1lastJSError = $1prevJSError;$n", [dollar])
   if p.target == targetJS:
     add(p.body, "} finally {" & tnl)
+    add(p.body, "framePtr = $1;$n" % [tmpFramePtr])
   if p.target == targetPHP:
     # XXX ugly hack for PHP codegen
     add(p.body, "}" & tnl)
@@ -1918,10 +1927,10 @@ proc frameCreate(p: PProc; procname, filename: Rope): Rope =
             procname, filename]
 
 proc frameDestroy(p: PProc): Rope =
-  result = rope(("framePtr = framePtr.prev;" | "$framePtr = $framePtr['prev'];") & tnl)
+  result = rope(("framePtr = F.prev;" | "$framePtr = $F['prev'];") & tnl)
 
 proc genProcBody(p: PProc, prc: PSym): Rope =
-  if optStackTrace in prc.options:
+  if hasFrameInfo(p):
     result = frameCreate(p,
               makeJSString(prc.owner.name.s & '.' & prc.name.s),
               makeJSString(toFilename(prc.info)))
@@ -1935,7 +1944,7 @@ proc genProcBody(p: PProc, prc: PSym): Rope =
   if prc.typ.callConv == ccSysCall and p.target == targetJS:
     result = ("try {$n$1} catch (e) {$n" &
       " alert(\"Unhandled exception:\\n\" + e.message + \"\\n\"$n}") % [result]
-  if optStackTrace in prc.options:
+  if hasFrameInfo(p):
     add(result, frameDestroy(p))
 
 proc genProc(oldProc: PProc, prc: PSym): Rope =
diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim
index 9c8a18bfe..abec44bbb 100644
--- a/lib/system/jssys.nim
+++ b/lib/system/jssys.nim
@@ -7,11 +7,6 @@
 #    distribution, for details about the copyright.
 #
 
-when defined(nodejs):
-  proc alert*(s: cstring) {.importc: "console.log", nodecl.}
-else:
-  proc alert*(s: cstring) {.importc, nodecl.}
-
 proc log*(s: cstring) {.importc: "console.log", varargs, nodecl.}
 
 type
@@ -91,9 +86,6 @@ proc auxWriteStackTrace(f: PCallFrame): string =
 proc rawWriteStackTrace(): string =
   if framePtr != nil:
     result = "Traceback (most recent call last)\n" & auxWriteStackTrace(framePtr)
-    framePtr = nil
-  elif lastJSError != nil:
-    result = $lastJSError.stack
   else:
     result = "No stack traceback available\n"
 
@@ -101,26 +93,33 @@ proc getStackTrace*(): string = rawWriteStackTrace()
 
 proc unhandledException(e: ref Exception) {.
     compilerproc, asmNoStackFrame.} =
-  when NimStackTrace:
-    var buf = rawWriteStackTrace()
+  var buf = ""
+  if e.msg != nil and e.msg[0] != '\0':
+    add(buf, "Error: unhandled exception: ")
+    add(buf, e.msg)
   else:
-    var buf = ""
-    if e.msg != nil and e.msg[0] != '\0':
-      add(buf, "Error: unhandled exception: ")
-      add(buf, e.msg)
-    else:
-      add(buf, "Error: unhandled exception")
-    add(buf, " [")
-    add(buf, e.name)
-    add(buf, "]\n")
-    alert(buf)
+    add(buf, "Error: unhandled exception")
+  add(buf, " [")
+  add(buf, e.name)
+  add(buf, "]\n")
+  when NimStackTrace:
+    add(buf, rawWriteStackTrace())
+  let cbuf : cstring = buf
+  framePtr = nil
+  {.emit: """
+  if (typeof(Error) !== "undefined") {
+    throw new Error(`cbuf`);
+  }
+  else {
+    throw `cbuf`;
+  }
+  """.}
 
 proc raiseException(e: ref Exception, ename: cstring) {.
     compilerproc, asmNoStackFrame.} =
   e.name = ename
-  when not defined(noUnhandledHandler):
-    if excHandler == 0:
-      unhandledException(e)
+  if excHandler == 0:
+    unhandledException(e)
   when defined(nimphp):
     asm """throw new Exception($`e`["message"]);"""
   else:
@@ -130,15 +129,15 @@ proc reraiseException() {.compilerproc, asmNoStackFrame.} =
   if lastJSError == nil:
     raise newException(ReraiseError, "no exception to reraise")
   else:
-    when not defined(noUnhandledHandler):
-      if excHandler == 0:
-        var isNimException: bool
-        when defined(nimphp):
-          asm "`isNimException` = isset(`lastJSError`['m_type']);"
-        else:
-          asm "`isNimException` = lastJSError.m_type;"
-        if isNimException:
-          unhandledException(cast[ref Exception](lastJSError))
+    if excHandler == 0:
+      var isNimException: bool
+      when defined(nimphp):
+        asm "`isNimException` = isset(`lastJSError`['m_type']);"
+      else:
+        asm "`isNimException` = lastJSError.m_type;"
+      if isNimException:
+        unhandledException(cast[ref Exception](lastJSError))
+
     asm "throw lastJSError;"
 
 proc raiseOverflow {.exportc: "raiseOverflow", noreturn.} =
@@ -873,3 +872,10 @@ proc nimParseBiggestFloat(s: string, number: var BiggestFloat, start = 0): int {
   # evaluate sign
   number = number * sign
   result = i - start
+
+when defined(nodejs):
+  # Deprecated. Use `alert` defined in dom.nim
+  proc alert*(s: cstring) {.importc: "console.log", nodecl, deprecated.}
+else:
+  # Deprecated. Use `alert` defined in dom.nim
+  proc alert*(s: cstring) {.importc, nodecl, deprecated.}
diff --git a/tests/exception/tunhandledexc.nim b/tests/exception/tunhandledexc.nim
index 63a402414..c318aec81 100644
--- a/tests/exception/tunhandledexc.nim
+++ b/tests/exception/tunhandledexc.nim
@@ -14,10 +14,9 @@ proc genErrors(s: string) =
     raise newException(EsomeotherErr, "bla")
 
 when true:
+  try: discard except: discard
+
   try:
     genErrors("errssor!")
   except ESomething:
     echo("Error happened")
-
-
-
diff --git a/tests/testament/categories.nim b/tests/testament/categories.nim
index 820078c54..29b6d9aba 100644
--- a/tests/testament/categories.nim
+++ b/tests/testament/categories.nim
@@ -221,6 +221,7 @@ proc jsTests(r: var TResults, cat: Category, options: string) =
   for testfile in ["exception/texceptions", "exception/texcpt1",
                    "exception/texcsub", "exception/tfinally",
                    "exception/tfinally2", "exception/tfinally3",
+                   "exception/tunhandledexc",
                    "actiontable/tactiontable", "method/tmultim1",
                    "method/tmultim3", "method/tmultim4",
                    "varres/tvarres0", "varres/tvarres3", "varres/tvarres4",
diff --git a/tests/testament/tester.nim b/tests/testament/tester.nim
index 83e59a6c1..74ac58927 100644
--- a/tests/testament/tester.nim
+++ b/tests/testament/tester.nim
@@ -334,6 +334,11 @@ proc testSpec(r: var TResults, test: TTest) =
 
     let exeCmd = (if isJsTarget: nodejs & " " else: "") & exeFile
     var (buf, exitCode) = execCmdEx(exeCmd, options = {poStdErrToStdOut})
+
+    # Treat all failure codes from nodejs as 1. Older versions of nodejs used
+    # to return other codes, but for us it is sufficient to know that it's not 0.
+    if exitCode != 0: exitCode = 1
+
     let bufB = if expected.sortoutput: makeDeterministic(strip(buf.string))
                else: strip(buf.string)
     let expectedOut = strip(expected.outp)
diff --git a/web/news/version_0_15_released.rst b/web/news/version_0_15_released.rst
index 2b1b216b8..683cbed0d 100644
--- a/web/news/version_0_15_released.rst
+++ b/web/news/version_0_15_released.rst
@@ -57,6 +57,11 @@ that have tuple name:
 - Now when you compile console application for Windows, console output
   encoding is automatically set to UTF-8.
 
+- Unhandled exceptions in JavaScript are now thrown regardless ``noUnhandledHandler``
+  is defined. But now they do their best to provide a readable stack trace.
+
+- In JavaScript ``system.alert`` is deprecated. Use ``dom.alert`` instead.
+
 Library Additions
 -----------------