Repository: elastic/synthetics (ts)

Issue:
Thrown exceptions causes steps to fail, but not the whole journey
If, to a journey, I try to fail a step with the code:

`throw("I ded")`

In the kibana UI I'll see a state where the step shows as failed, but the overall journey / check shows as passed.

If I use a playwright assertion, such as

`expect("I ded").toBe("ded")`

Both will fail as expected

Pull request:
fix: properly handle non error exceptions
Catch non `Error` objects that are thrown as exceptions. As a quick workaround devs sometimes throw using syntax like `throw("foo")` instead of the more correct `throw(new Error("foo"))`. 

The current code will swallow those errors.

Fixes #692 

## Testing

This PR includes a simple unit test. Given that the impact of #692 is low, and writing a true E2E test demonstrating the resolution of that issue, I think manual testing of that scenario is fine.

To do so, save the following file as `bad.ts`

```ts
//bad.ts

step("instant exception", () => {
  throw("simple exception");
})
```

Then, run 

```sh
npm run build && node dist/cli.js --inline --rich-events < bad.ts | grep journey/end |  jq .error`
```

after checking out this branch. You should see this error. Switching back to `main` the error should be missing.

Patch:
diff --git a/src/helpers.ts b/src/helpers.ts
index e33193d..7f3f97b 100644
--- a/src/helpers.ts
+++ b/src/helpers.ts
@@ -233,10 +233,21 @@ export function rewriteErrorStack(stack: string, indexes: [number, number]) {
   return stack;
 }
 
-export function formatError(error: Error) {
-  if (!(error instanceof Error)) {
+// formatError prefers receiving proper Errors, but since at runtime
+// non Error exceptions can be thrown, it tolerates though. The
+// redundant type Error | any expresses that.
+export function formatError(error: Error | any) {
+  if (error === undefined || error === null) {
     return;
   }
+
+  if (!(error instanceof Error)) {
+    return {
+      message: `Error "${error}" received, with type "${typeof error}". (Do not throw exceptions without using \`new Error("my message")\`)`,
+      name: '',
+      stack: '',
+    };
+  }
   const { name, message, stack } = error;
   const indexes = findPWLogsIndexes(message);
   return {


Test patch:
diff --git a/__tests__/helper.test.ts b/__tests__/helper.test.ts
index 1e43b08..721f2e0 100644
--- a/__tests__/helper.test.ts
+++ b/__tests__/helper.test.ts
@@ -57,14 +57,32 @@ it('convert trace timestamp to internal time', () => {
   expect(elapsedTime).toBe(392583.998697);
 });
 
-it('format errors', () => {
-  const error = new Error('testing');
-  const { name, message, stack } = error;
-  const formatted = formatError(error);
-  expect(formatted).toStrictEqual({
-    name,
-    message,
-    stack,
+describe('formatting errors', () => {
+  it('formats proper errors', () => {
+    const error = new Error('testing');
+    const { name, message, stack } = error;
+    const formatted = formatError(error);
+    expect(formatted).toStrictEqual({
+      name,
+      message,
+      stack,
+    });
+  });
+
+  ["c'est ne pas une Error", 42, { an: 'object' }].forEach(obj => {
+    it(`formats thrown non-error errors like: ${obj}(${typeof obj})`, () => {
+      const formatted = formatError(obj)!;
+      expect(formatted.message).toContain(`${obj}`);
+      expect(formatted.name).toStrictEqual('');
+      expect(formatted.stack).toStrictEqual('');
+      expect(typeof formatted.message).toBe('string');
+    });
+  });
+
+  [null, undefined].forEach(obj => {
+    it(`returns undefined for ${typeof obj}`, () => {
+      expect(formatError(obj)).toBe(undefined);
+    });
   });
 });
 
diff --git a/__tests__/plugins/network.test.ts b/__tests__/plugins/network.test.ts
index 74f4326..fbcf25a 100644
--- a/__tests__/plugins/network.test.ts
+++ b/__tests__/plugins/network.test.ts
@@ -281,7 +281,8 @@ describe('network', () => {
       const timing = res.timings;
       expect(timing.total).toBeGreaterThanOrEqual(timing.wait + timing.receive);
       // Check with absolute value to make sure we are not crossing absurd values
-      expect(timing.total).toBeLessThan(50);
+      // can be somewhat high on CI
+      expect(timing.total).toBeLessThan(100);
     });
   });