From 033d5cfe862d8cc453f3acbd2f4abc6e5ed2359a Mon Sep 17 00:00:00 2001 From: "Randall C. O'Reilly" Date: Sun, 25 Oct 2026 13:40:57 +0100 Subject: [PATCH] fix(builtin): recover JavaScript errors thrown as panics recover only stopped a GoPanic, so a panic that runtime code throws as a JavaScript error, such as a reflect.ValueError or a TypeError, could not be recovered, and unwound past every deferred recover. Such an error now unwinds as a panic whose value is the error, when it has an Error method, or its message. The sentinels of runtime.Goexit and os.Exit still are not recovered. reflect.ValueError gets its Error method and is registered, so a recovered *reflect.ValueError asserts as one, and a call on the zero Value says so as Go does. Signed-off-by: Randall C. O'Reilly --- gs/builtin/panic.test.ts | 49 +++++++++++++- gs/builtin/panic.ts | 36 +++++++++- gs/reflect/types.ts | 37 +++++++++- .../reflect_recover_value_error/expected.log | 2 + .../reflect_recover_value_error/index.ts | 0 .../reflect_recover_value_error.go | 23 +++++++ .../reflect_recover_value_error.gs.ts | 67 +++++++++++++++++++ .../reflect_recover_value_error/tsconfig.json | 25 +++++++ 8 files changed, 232 insertions(+), 7 deletions(-) create mode 100644 tests/tests/reflect_recover_value_error/expected.log create mode 100644 tests/tests/reflect_recover_value_error/index.ts create mode 100644 tests/tests/reflect_recover_value_error/reflect_recover_value_error.go create mode 100644 tests/tests/reflect_recover_value_error/reflect_recover_value_error.gs.ts create mode 100644 tests/tests/reflect_recover_value_error/tsconfig.json diff --git a/gs/builtin/panic.test.ts b/gs/builtin/panic.test.ts index 18c3cc15b..f6c0920dc 100644 --- a/gs/builtin/panic.test.ts +++ b/gs/builtin/panic.test.ts @@ -15,7 +15,6 @@ import { import { goSlice, index, makeSlice } from './slice.js' import { DisposableStack } from './defer.js' - // captureThrow returns whatever value the callback throws, or undefined if it // did not throw. Go panics surface as a thrown GoPanic in the transpiled // runtime, so tests assert on the caught value's shape. Catching here models a @@ -193,6 +192,54 @@ describe('recover consumes the in-flight panic', () => { ) }) + it('recovers a JavaScript error thrown without panic', () => { + // An error with an Error method is the value, as reflect.ValueError. + class valueError extends Error { + Error(): string { + return this.message + } + } + const thrown = new valueError('reflect: call on zero Value') + let seen: unknown = 'unset' + const outcome = runWithRecover( + () => { + throw thrown + }, + () => { + seen = recover() + }, + ) + expect(seen).toBe(thrown) + expect(outcome.finished).toBe(true) + + // Another error is its message. + runWithRecover( + () => { + throw new TypeError('x is undefined') + }, + () => { + seen = recover() + }, + ) + expect(seen).toBe('x is undefined') + }) + + it('does not recover runtime.Goexit', () => { + const goexit = new Error('runtime.Goexit') + goexit.name = 'GoexitError' + let seen: unknown = 'unset' + const outcome = runWithRecover( + () => { + throw goexit + }, + () => { + seen = recover() + }, + ) + expect(seen).toBeNull() + expect(outcome.rethrew).toBe(goexit) + }) + it('normal defer disposal cannot recover an unrelated panic', () => { let seen: unknown = 'unset' try { diff --git a/gs/builtin/panic.ts b/gs/builtin/panic.ts index 63e1db187..a43609bdb 100644 --- a/gs/builtin/panic.ts +++ b/gs/builtin/panic.ts @@ -30,7 +30,7 @@ let activeRecoverPanic: GoPanic | null = null export function withRecoveringPanic(err: unknown, fn: () => T): T { const previous = activeRecoverPanic - activeRecoverPanic = err instanceof GoPanic ? err : null + activeRecoverPanic = asGoPanic(err) try { return fn() } finally { @@ -38,6 +38,36 @@ export function withRecoveringPanic(err: unknown, fn: () => T): T { } } +// errorPanics holds the GoPanic that stands for a JavaScript error thrown +// without panic, such as a reflect.ValueError of the reflect package or a +// TypeError, which Go code recovers as it does a panic. It is keyed by the +// error, which the generated catch block passes to recovered. +const errorPanics = new WeakMap() + +// asGoPanic returns the GoPanic that err unwinds as, or null if err is not +// a panic: runtime.Goexit and os.Exit throw errors that recover does not +// stop. The value of a panic from an error without an Error method is the +// error's message. +function asGoPanic(err: unknown): GoPanic | null { + if (err instanceof GoPanic) { + return err + } + if ( + !(err instanceof Error) || + err.name === 'GoexitError' || + err.name === 'ProcessExitError' + ) { + return null + } + let goPanic = errorPanics.get(err) + if (goPanic === undefined) { + const hasError = typeof (err as { Error?: unknown }).Error === 'function' + goPanic = new GoPanic(hasError ? err : err.message) + errorPanics.set(err, goPanic) + } + return goPanic +} + // RuntimeError is the value carried by a panic from a Go runtime fault: index // out of range, slice bounds out of range, integer divide by zero, or nil // pointer dereference. It implements the Go error interface (Error()), so a @@ -129,11 +159,11 @@ export function recover(): any { return goPanic.value } -// recovered reports whether err is a GoPanic that a deferred recover() consumed. +// recovered reports whether err is a panic that a deferred recover() consumed. // The catch block generated for a // defer+recover function calls this: true means swallow the panic and return the // function's named results; false means rethrow so an outer frame can recover or // the program crashes with the Go panic message. export function recovered(err: unknown): boolean { - return err instanceof GoPanic && err.recovered + return asGoPanic(err)?.recovered === true } diff --git a/gs/reflect/types.ts b/gs/reflect/types.ts index ed272bdc5..08be5c017 100644 --- a/gs/reflect/types.ts +++ b/gs/reflect/types.ts @@ -286,11 +286,42 @@ export class ValueError extends Error { public Method: string constructor(init: { Kind: Kind; Method: string }) { - super( - `reflect: call of reflect.Value.${init.Method} on ${Kind_String(init.Kind)} Value`, - ) + // Go calls a Value of kind Invalid the zero Value. + const kind = init.Kind === 0 ? 'zero' : Kind_String(init.Kind) + super(`reflect: call of reflect.Value.${init.Method} on ${kind} Value`) this.Kind = init.Kind this.Method = init.Method this.name = 'ValueError' } + + // Error returns the message, as the Error method of *ValueError. + Error(): string { + return this.message + } + + // Register the type, so that a recovered *ValueError asserts as one. + static __typeInfo = $.registerStructType( + 'reflect.ValueError', + null, + [ + { + name: 'Error', + args: [], + returns: [{ type: { kind: $.TypeKind.Basic, name: 'string' } }], + }, + ], + ValueError, + [ + { + name: 'Kind', + key: 'Kind', + type: { kind: $.TypeKind.Basic, name: 'int' }, + }, + { + name: 'Method', + key: 'Method', + type: { kind: $.TypeKind.Basic, name: 'string' }, + }, + ], + ) } diff --git a/tests/tests/reflect_recover_value_error/expected.log b/tests/tests/reflect_recover_value_error/expected.log new file mode 100644 index 000000000..9ef9f6961 --- /dev/null +++ b/tests/tests/reflect_recover_value_error/expected.log @@ -0,0 +1,2 @@ +ok +recovered: true reflect: call of reflect.Value.Field on zero Value diff --git a/tests/tests/reflect_recover_value_error/index.ts b/tests/tests/reflect_recover_value_error/index.ts new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests/reflect_recover_value_error/reflect_recover_value_error.go b/tests/tests/reflect_recover_value_error/reflect_recover_value_error.go new file mode 100644 index 000000000..38deed5f2 --- /dev/null +++ b/tests/tests/reflect_recover_value_error/reflect_recover_value_error.go @@ -0,0 +1,23 @@ +package main + +import ( + "fmt" + "reflect" +) + +// field returns the first field of v, or the panic it recovers. +func field(v reflect.Value) (s string) { + defer func() { + if r := recover(); r != nil { + _, isErr := r.(*reflect.ValueError) + s = fmt.Sprint("recovered: ", isErr, " ", r) + } + }() + return v.Field(0).String() +} + +func main() { + type T struct{ S string } + println(field(reflect.ValueOf(T{S: "ok"}))) + println(field(reflect.Value{})) +} diff --git a/tests/tests/reflect_recover_value_error/reflect_recover_value_error.gs.ts b/tests/tests/reflect_recover_value_error/reflect_recover_value_error.gs.ts new file mode 100644 index 000000000..110d018dd --- /dev/null +++ b/tests/tests/reflect_recover_value_error/reflect_recover_value_error.gs.ts @@ -0,0 +1,67 @@ +// Generated file based on reflect_recover_value_error.go +// Updated when compliance tests are re-run, DO NOT EDIT! + +import * as $ from "@goscript/builtin/index.js" +import * as fmt from "@goscript/fmt/index.js" +import * as reflect from "@goscript/reflect/index.js" + +export function field(v: reflect.Value): string { + let s = "" + const __defer = new $.DisposableStack() + try { + __defer.defer(() => { (() => { + { + let r = $.recover() + if (r != null) { + let [, isErr] = $.typeAssertTuple | null>(r, /* @__PURE__ */ $.pointerType("reflect.ValueError")) + s = (fmt.Sprint("recovered: ", isErr, " ", r) as string) + } + } + })() }) + const __goscriptReturn0: string = $.markAsStructValue($.cloneStructValue($.markAsStructValue($.cloneStructValue(v)).Field(0))).String() + s = __goscriptReturn0 + __defer.dispose() + return s + __defer.dispose() + } catch (e) { + __defer.disposePanic(e) + if (!$.recovered(e)) { + throw e + } + } + return s +} + +export async function main(): globalThis.Promise { + class T { + public declare S: string + + constructor(init?: Partial<{S?: string}>, __goscriptValue?: boolean) { + this.S = init?.S ?? ("" as string) + ;(this as any)[$.structValueMarker] = __goscriptValue === true + } + + public clone(): T { + return new T(this, true) + } + + public __goscriptAssign(source: T): this { + this.S = source.S + return this + } + + static __typeInfo = $.registerStructType( + "main.T", + () => new T(), + () => [], + T, + () => [/* @__PURE__ */ $.structField("S", /* @__PURE__ */ $.basicType("string"), [0], 0, true)] + ) + } + await $.println(field($.markAsStructValue($.cloneStructValue(reflect.ValueOf($.structInterfaceValue($.markAsStructValue(new T({S: "ok"})), "main.T", "main.T")))))) + await $.println(field($.markAsStructValue(new reflect.Value()))) +} + +if ($.isMainScript(import.meta)) { + await main() +} diff --git a/tests/tests/reflect_recover_value_error/tsconfig.json b/tests/tests/reflect_recover_value_error/tsconfig.json new file mode 100644 index 000000000..90615133f --- /dev/null +++ b/tests/tests/reflect_recover_value_error/tsconfig.json @@ -0,0 +1,25 @@ +{ + "compilerOptions": { + "paths": { + "*": [ + "./*" + ], + "@go/github.com/s4wave/goscript/*": [ + "../../../*" + ], + "@goscript/*": [ + "../../../tests/tests/reflect_recover_value_error/run/output/@goscript/*", + "../../../gs/*", + "../../../tests/deps/*" + ], + "@goscript/github.com/s4wave/goscript/tests/tests/reflect_recover_value_error/*": [ + "./*" + ] + } + }, + "extends": "../../../tests/tsconfig.base.json", + "include": [ + "index.ts", + "reflect_recover_value_error.gs.ts" + ] +}