Handle thrown values that are not Errors

SKILL.md

@@ -71,7 +71,7 @@ All properties are optional and inherit from parent to child.
 | `args`      | Array of arguments passed to `run`. Non-arrays auto-wrapped. `arg` takes precedence                                                                                                                                  |
 | `expect`    | Expected result. Deep equality by default                                                                                                                                                                             |
 | `getExpect` | Function to generate expected value dynamically. Called like `run`: `getExpect.apply(test, args)`. Inherited. `expect` takes precedence if both are set. If the getter throws, falls through to default (`args[0]`) |
-| `throws`    | `true` (any error), `false` (asserts no error thrown), Error subclass (`TypeError`), or predicate `e => e.code === "ENOENT"`. Inherited                                                                               |
+| `throws`    | `true` (any error), `false` (asserts no error thrown), Error subclass (`TypeError`), or predicate `e => e.code === "ENOENT"`. Inherited. A thrown non-Error (e.g. a string) is wrapped in an `Error`, with the original on `.cause`                                                                               |
 
 ### Structure
 

src/classes/TestResult.js

@@ -1,7 +1,7 @@
 import Test from "./Test.js";
 import BubblingEventTarget from "./BubblingEventTarget.js";
 import { stripFormatting } from "../format-console.js";
-import { delay, formatDuration, interceptConsole, pluralize, stringify } from "../util.js";
+import { asError, delay, formatDuration, interceptConsole, pluralize, stringify } from "../util.js";
 import { formatDiff } from "../util/format-diff.js";
 
 /**
@@ -86,6 +86,7 @@ export default class TestResult extends BubblingEventTarget {
 					await this.test.beforeAll?.();
 				}
 				catch (e) {
+					e = asError(e);
 					e.source = "beforeAll";
 					this.error = error = e;
 				}
@@ -96,6 +97,7 @@ export default class TestResult extends BubblingEventTarget {
 					await this.test.beforeEach?.apply(this.test, this.test.args);
 				}
 				catch (e) {
+					e = asError(e);
 					e.source = "beforeEach";
 					this.error = error = e;
 				}
@@ -115,6 +117,8 @@ export default class TestResult extends BubblingEventTarget {
 					}
 				}
 				catch (e) {
+					e = asError(e);
+
 					// Duck-type assertion errors (Node assert, Chai, etc.) — use their actual/expected for diffs
 					if ("actual" in e) {
 						this.actual = e.actual;
@@ -128,6 +132,7 @@ export default class TestResult extends BubblingEventTarget {
 				await this.test.afterEach?.apply(this.test, this.test.args);
 			}
 			catch (e) {
+				e = asError(e);
 				e.source = "afterEach";
 				this.error ??= e;
 				error ??= e;
@@ -139,6 +144,7 @@ export default class TestResult extends BubblingEventTarget {
 					await this.test.afterAll?.();
 				}
 				catch (e) {
+					e = asError(e);
 					e.source = "afterAll";
 					this.error ??= e;
 					error ??= e;
@@ -192,6 +198,7 @@ export default class TestResult extends BubblingEventTarget {
 						await this.test.beforeAll?.();
 					}
 					catch (e) {
+						e = asError(e);
 						e.source = "beforeAll";
 						error = e;
 					}
@@ -331,12 +338,14 @@ export default class TestResult extends BubblingEventTarget {
 					ret.pass = test.check(this.mapped.actual, this.mapped.expect);
 				}
 				catch (e) {
+					e = asError(e);
 					this.error = new Error(
 						`check() failed (working with mapped values). ${e.message}`,
 					);
 				}
 			}
 			catch (e) {
+				e = asError(e);
 				this.error = new Error(`map() failed. ${e.message}`);
 			}
 		}
@@ -345,6 +354,7 @@ export default class TestResult extends BubblingEventTarget {
 				ret.pass = test.check(this.actual, test.expect);
 			}
 			catch (e) {
+				e = asError(e);
 				this.error = new Error(`check() failed. ${e.message}`);
 			}
 		}

src/util.js

@@ -5,6 +5,16 @@ import * as objects from "./objects.js";
  */
 export const IS_NODEJS = typeof process === "object" && process?.versions?.node;
 
+/**
+ * Make a thrown value safe to annotate and to read `.message` and `.stack` from, since anything at
+ * all can be thrown. Objects pass through; primitives are wrapped, with the original kept as `cause`.
+ * @param {*} value
+ * @returns {Error | object}
+ */
+export function asError (value) {
+	return Object(value) === value ? value : new Error(String(value), { cause: value });
+}
+
 /**
  * Determine the internal JavaScript [[Class]] of an object.
  * @param {*} value - Value to check

tests/errors.js

@@ -74,6 +74,16 @@ export default {
 						expect: "foo",
 					},
 				},
+				{
+					name: "Thrown value is not an Error (issue #183)",
+					arg: {
+						beforeEach () {
+							throw "boom";
+						},
+						arg: "foo",
+						expect: "foo",
+					},
+				},
 			],
 		},
 		{
@@ -266,6 +276,54 @@ export default {
 				},
 			],
 		},
+		{
+			name: "Non-Error thrown values (issue #183)",
+			description: "Throwing a primitive is legal JS, and must not take down the runner.",
+			async run (test) {
+				let result = await runTest(test);
+				return result.pass;
+			},
+			tests: [
+				{
+					name: "Reports the value, with no phantom stack line",
+					description: "A primitive has no .stack, so interpolating one appends a bare `undefined`.",
+					async run (test) {
+						let result = await runTest(test);
+						return result.details[0];
+					},
+					arg: {
+						run () {
+							throw "boom";
+						},
+						expect: 1,
+					},
+					check: (actual, expect) => actual.includes(expect) && !actual.includes("undefined"),
+					expect: "boom",
+				},
+				{
+					name: "Keeps the thrown value as cause",
+					description: "Wrapping is what makes .message and .stack safe to read; the original must survive it.",
+					arg: {
+						run () {
+							throw "boom";
+						},
+						throws: error => error.cause === "boom",
+					},
+					expect: true,
+				},
+				{
+					name: "A falsy value still counts as a throw",
+					description: "Otherwise throws: false passes for a test that did throw.",
+					arg: {
+						run () {
+							throw null;
+						},
+						throws: false,
+					},
+					expect: false,
+				},
+			],
+		},
 		{
 			name: "AssertionError treated as failure (issue #114)",
 			skip: typeof globalThis.process === "undefined",