toThrowError() Does Not Support Custom Error Types

[kmanev073]

Describe the bug

When a custom object-type error is thrown and a string parameter is passed to toThrowError() an unhanded exception is thrown:

TypeError: Cannot read properties of undefined (reading 'indexOf')
 ❯ Object.compatibleMessage node_modules/check-error/index.js:83:29

I guess vitest expects to always receive an error with a message property that has a indexOf function. Well... this seems that this is not always the case.

This happens with normal and async functions as well.

I found the issue while I was trying to test SvelteKit application where I use the error() (link) function which throws a custom HttpError (link).

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-bdugz4?file=test%2Fbasic.test.ts

I've seen this issue issue 1636 but it doesn't explain on how to assert the contents of the object instance that was thrown.

System Info

System:
    OS: Windows 11 10.0.22635
    CPU: (8) x64 Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz
    Memory: 16.11 GB / 31.76 GB
  Binaries:
    Node: 18.18.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.17 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.8.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (120.0.2210.7)
    Internet Explorer: 11.0.22621.1

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[sheremet-va]

This is an error on chai.js side, - toThrowError is an alias for expect().throws()

[hi-ogawa]

how to assert the contents of the object instance that was thrown.

I'm not sure if this is a legitimate workaround, but I found this async/rejects trick to achieve something like this:

https://stackblitz.com/edit/vitest-dev-vitest-vk98np?file=test%2Fbasic.test.ts

test('extract thrown object via async/rejects', async () => {
  const func = () => {
    throw {
      body: {
        message: 'Custom error type',
      },
    };
  };

  await expect(async () => func()).rejects.toMatchInlineSnapshot(`
    {
      "body": {
        "message": "Custom error type",
      },
    }
  `);
});

I feel there should be non-async version of this pattern, but I'm not sure Jest has such assertion either.

[hi-ogawa]

I feel there should be non-async version of this pattern

I was experimenting with such custom assertion. I think this could be implemented in user-land as chai plugin and here is my attempt:

https://stackblitz.com/edit/vitest-dev-vitest-c1bw5j?file=test%2Fbasic.test.ts

reveal code

import chai from 'chai';
import { describe, expect, it } from 'vitest';

declare module 'vitest' {
  interface Assertion<T = any> {
    thrownValue: Assertion<T>;
  }
}

chai.use((chai, utils) => {
  utils.addProperty(
    chai.Assertion.prototype,
    'thrownValue',
    function __VITEST_REJECTS__(this: any) {
      const error = new Error('thrownValue');
      utils.flag(this, 'error', error);
      const obj = utils.flag(this, 'object');

      if (typeof obj !== 'function')
        throw new TypeError(
          `You must provide a function to expect() when using .throws2, not '${typeof obj}'.`
        );

      const proxy: any = new Proxy(this, {
        get: (target, key, receiver) => {
          const result = Reflect.get(target, key, receiver);

          if (typeof result !== 'function')
            return result instanceof chai.Assertion ? proxy : result;

          return (...args: any[]) => {
            let value: any;
            try {
              value = obj();
            } catch (err) {
              utils.flag(this, 'object', err);
              return result.call(this, ...args);
            }
            const _error = new chai.AssertionError(
              `function returned "${utils.inspect(value)}" instead of throwing`,
              { showDiff: true, expected: new Error('throws'), actual: value }
            ) as any;
            _error.stack = (error.stack as string).replace(
              error.message,
              _error.message
            );
            throw _error;
          };
        },
      });

      return proxy;
    }
  );
});

//
// example usage
//

describe('thrownValue', () => {
  it('basic', () => {
    // this would be mostly same as builtin `toThrowErrorMatchingInlineSnapshot`
    expect(() => {
      throw new Error('hello');
    }).thrownValue.toMatchInlineSnapshot('[Error: hello]');

    expect(() => {
      throw new Error('hello');
    }).toThrowErrorMatchingInlineSnapshot(`[Error: hello]`);

    // but this allows asserting other than "message"
    expect(() => {
      throw new Error('hello', { cause: 'world' });
    }).thrownValue.toMatchObject({ cause: 'world' });

    // also non Error instance
    expect(() => {
      throw { some: 'object' };
    }).thrownValue.toMatchObject({ some: expect.stringContaining('j') });
  });

  it('expect to throw', () => {
    expect(() => {
      expect(() => 'all good').thrownValue.toBeDefined();
    }).toThrowErrorMatchingInlineSnapshot(
      `[AssertionError: function returned "'all good'" instead of throwing]`
    );
  });
});

I mostly followed how vitest implements rejects in:

vitest/packages/expect/src/jest-expect.ts

Line 732 in 0802167

utils.addProperty(chai.Assertion.prototype, 'rejects', function __VITEST_REJECTS__(this: any) {

[xixixao]

Directly changing this would probably be a breaking change, but I agree with this comment:

jestjs/jest#8140 (comment)

toThrow() should simply be chainable, instead of supporting one of a few ways to assert what the error is.

Another "fix" i'd be happy with is for toThrowError to take in a function, which can then do whatever it wants with the error (including expect() calls). This can be done with try and catch but it's super onerous (making sure the catch is clause is actually invoked).

[hi-ogawa]

Thanks for referencing the issue from Jest. I've been also wondering whether Jest has considered chain-able synchronous throw at some point.

Somehow rejects is already chain-able "asynchronous", so I felt it would be natural to want to have synchronous version of it. What I prototyped above thrownValue is my quick synchronous adaptation of rejects.

Another "fix" i'd be happy with is for toThrowError to take in a function, which can then do whatever it wants with the error

Node's assert has this feature https://nodejs.org/api/assert.html#assertthrowsfn-error-message and I agree this is quite ergonomic.

For chainable things, this can be already achieved by toSatisfy https://vitest.dev/api/expect.html#tosatisfy and I use this pattern sometimes, for example https://github.com/hi-ogawa/js-utils/blob/70cb40574c6b9a53d77cef4080fb2d039c34180b/packages/tiny-rpc/src/adapter-http.test.ts#L85-L122

await expect(() => ...).rejects.toSatisfy((e) => {
   expect(e).toMatchInlineSnapshot(`...`);
   expect(e.cause).toMatchInlineSnapshot(`...`);
   return true
});

---

Well, looking back the content of the original issue, it's not really about the chainable throw, so probably I should've created a separate feature request.

[carlschroedl]

@kmanev073 Could this use case be supported by modifying toThrowError to 1. accept a class as a parameter and 2. verify that the thrown error is an instance of that class?

Here's an untested sketch demonstrating ...

// implementation.js
export class HttpError extends Error {}

export function myFunction() {
  throw new HttpError()
}

// test.js
import { HttpError, myFunction } from './implementation.js' 
expect(myFunction()).toThrowError(HttpError)

In other words, should toThrowError run something like isClass, on its argument, and if the value is true, check to see if the thrown error is an instanceof the parameterized class?