vitest icon indicating copy to clipboard operation
vitest copied to clipboard

toThrowError() Does Not Support Custom Error Types

Open kmanev073 opened this issue 2 years ago • 6 comments

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

kmanev073 avatar Nov 20 '23 21:11 kmanev073

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

sheremet-va avatar Nov 27 '23 13:11 sheremet-va

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 avatar Nov 28 '23 05:11 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:

https://github.com/vitest-dev/vitest/blob/08021673df80958552f4c3717ef03fd11f46aee5/packages/expect/src/jest-expect.ts#L732

hi-ogawa avatar Dec 05 '23 06:12 hi-ogawa

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

https://github.com/jestjs/jest/issues/8140#issuecomment-557895399

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).

xixixao avatar Apr 09 '24 14:04 xixixao

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.

hi-ogawa avatar Apr 09 '24 23:04 hi-ogawa

@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?

carlschroedl avatar Aug 09 '24 21:08 carlschroedl

I reported an OP's issue on chai https://github.com/chaijs/chai/issues/1752.

For the purpose of asserting on a thrown error, I suggest to use expect.toSatisfy matcher, which allows an arbitrary customization and works similar to unwrapping thrown error https://stackblitz.com/edit/vitest-dev-vitest-8eqgdqsk?file=test%2Fbasic.test.ts

import { expect, test } from 'vitest';

test('should fail', async () => {
  const func = () => {
    throw {
      body: {
        message: 'Custom error type',
      },
    };
  };

  expect(() => func()).toThrowError(expect.toSatisfy(error => {
    return error.body?.message === 'Custom error type';
  }));
});

This even allows rather exotic:

  expect(() => func()).toThrowError(expect.toSatisfy(error => {
    expect(error).toMatchInlineSnapshot(`
      {
        "body": {
          "message": "Custom error type",
        },
      }
    `)
    return true
  }));

For the purpose of OP's intent, this seems sufficient so let me close this. If anyone has a further request, please create a new issue.

hi-ogawa avatar Nov 27 '25 10:11 hi-ogawa