From 90b0a771e405eab9bc48e83ca7e4f47d3203d088 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 14:11:41 +0300 Subject: [PATCH 1/7] Uncaught exceptions in JS now always propagate with better stack trace. --- lib/system/jssys.nim | 66 +++++++++++++++++------------- web/news/version_0_15_released.rst | 3 ++ 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim index 9c8a18bfe..0fbf4944a 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 @@ -101,26 +96,32 @@ 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 + {.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 +131,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 +874,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/web/news/version_0_15_released.rst b/web/news/version_0_15_released.rst index 2b1b216b8..bdbd79a2b 100644 --- a/web/news/version_0_15_released.rst +++ b/web/news/version_0_15_released.rst @@ -57,6 +57,9 @@ 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. + Library Additions ----------------- From 8bff930c7c0bfa02e4d942faec1e7532d85f0cba Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 15:08:03 +0300 Subject: [PATCH 2/7] Fixed js codegen issue for uncaught exceptions. --- compiler/jsgen.nim | 3 ++- tests/exception/tunhandledexc.nim | 5 ++--- tests/testament/categories.nim | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim index 80bcd2b0e..335ee2b92 100644 --- a/compiler/jsgen.nim +++ b/compiler/jsgen.nim @@ -560,6 +560,7 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) = # ++excHandler; # try { # stmts; + # --excHandler; # } catch (EXC) { # var prevJSError = lastJSError; lastJSError = EXC; # --excHandler; @@ -595,7 +596,7 @@ 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", []) elif p.target == targetPHP: addf(p.body, "} catch (Exception $$EXC) {$n $$prevJSError = $$lastJSError;$n $$lastJSError = $$EXC;$n", []) 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 e534cc161..8d8d0bcec 100644 --- a/tests/testament/categories.nim +++ b/tests/testament/categories.nim @@ -220,6 +220,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", From 62785be5a5e1ea9b74e40fcaed85ead09c7535d4 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 18:09:24 +0300 Subject: [PATCH 3/7] Restore framePtr after exceptions --- compiler/jsgen.nim | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim index 335ee2b92..357dd25ed 100644 --- a/compiler/jsgen.nim +++ b/compiler/jsgen.nim @@ -558,11 +558,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; @@ -573,6 +575,7 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) = # } # lastJSError = prevJSError; # } finally { + # framePtr = tmpFramePtr; # stmts; # } genLineDir(p, n) @@ -585,8 +588,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 @@ -598,6 +603,7 @@ proc genTry(p: PProc, n: PNode, r: var TCompRes) = if p.target == targetJS and catchBranchesExist: 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: @@ -619,8 +625,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", []) @@ -632,6 +637,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) From c1dd65c01a7c94764d8a4c255541136b95e643b7 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 18:10:13 +0300 Subject: [PATCH 4/7] Reset framePtr on unhabdled exception. Dont reset on getStackTrace. --- lib/system/jssys.nim | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim index 0fbf4944a..abec44bbb 100644 --- a/lib/system/jssys.nim +++ b/lib/system/jssys.nim @@ -86,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" @@ -108,6 +105,7 @@ proc unhandledException(e: ref Exception) {. when NimStackTrace: add(buf, rawWriteStackTrace()) let cbuf : cstring = buf + framePtr = nil {.emit: """ if (typeof(Error) !== "undefined") { throw new Error(`cbuf`); From e39138fe422b256c1bb08235ec2331f58c1449b6 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 19:33:34 +0300 Subject: [PATCH 5/7] Fixed codegen for asmNoStackFrame pragma. More bulletproof frame restoration. --- compiler/jsgen.nim | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim index 357dd25ed..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) = @@ -1925,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))) @@ -1942,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 = From cafcf1308a608ce35aa3a7abba8a28534e2b75e1 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 20:25:36 +0300 Subject: [PATCH 6/7] Treat all failure codes from nodejs as 1 --- tests/testament/tester.nim | 5 +++++ 1 file changed, 5 insertions(+) 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) From fc4bced41b174668c5239afda1d5aade45f055bf Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Sat, 27 Aug 2016 21:37:02 +0300 Subject: [PATCH 7/7] Added a note about system.alert deprecation --- web/news/version_0_15_released.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/news/version_0_15_released.rst b/web/news/version_0_15_released.rst index bdbd79a2b..683cbed0d 100644 --- a/web/news/version_0_15_released.rst +++ b/web/news/version_0_15_released.rst @@ -60,6 +60,8 @@ that have tuple name: - 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 -----------------