errorLike constructor can create misleading error

[dcramer]

I'll caveat all this with I don't know how everythings wired up, so consider this just a point of user feedback.

Vitest was grabbing an unhandled promise rejection, as shown by the second error in this screen grab:

The issue is this was getting masked by the new errorLike() call inside of check-error. Whether the call is correct or not I'll leave to someone who knows better, but from a usercode land I'm simply doing a typical throw new Error pattern.

Changing this code (to debug, temporarily) to simply be errorLike(), I was able to grab the actual error:

its not obvious to me which package is to blame for the behavior, so apologies if its not here, but as an end-user this was quite frustrating to debug.

For some additional clarity, afaict:

• check-error did not change at all afaict
• chai was upgraded (I believe from 4.3.10 to 4.4.1)
• vitest was upgraded

I'm using lockfiles and given implicit dependencies its a bit hard to bisect, as e.g. downgrading vitest on its own did not resolve this. Will try to debug and update if I can trace to true root cause.

[dcramer]

Found this which is related vitest-dev/vitest#5285

[43081j]

im guessing the error being thrown isn't an instance of anything

do you know what exactly a TRPCError is? it sounds like its a regular function

either way, chai itself should probably check that the errorLike is actually a constructor before trying to get the constructor name, somehow. this does seem like a bug

[dcramer]

Should be a normal error (its generated by the trpc framework). I've not actually diagnosed whats going on, but atm I'm trying to remove the promise rejections from tests to at least move past this issue myself.

Mostly the issue is just that its one thing masking another error, which almost certainly is an upstream problem, I just couldn't really trace where/how everythings used. To me it'd make sense if chai (which I believe is what calls this?) could handle its own error there and rely on Error.cause.

[43081j]

it does seem like a bug but in chai itself rather than check-error, basically.

when you call toThrow, chai expects the arg to be an Error instance or a class which produces one when newed up.

your error is being covered up because check-error is unexpectedly erroring over the top of it (thanks to the above).

could you let us know what exactly errorLike is? you can just console.log it or something

i've tried the following and it works fine:

import {TRPCError} from '@trpc/server';
import {getConstructorName} from 'check-error';
console.log(getConstructorName(TRPCError));

which suggests there's something in your case i'm missing

also if you could share the exact line of code that calls toThrow, that would be helpful

[dcramer]

fyi works fine on TRPCError, but injecting into error-like itself to grab the value:

{ errorLike: [Function (anonymous)] }

This comes from the rejects test, both all thats happening is caller.bottleCreate() is throw'ing a TRPCError instance.

test("_", async () => {
  const caller = createCaller({ user: null });
  expect(
    caller.bottleCreate({
      name: "Delicious Wood",
      brand: {
        name: "Hard Knox",
        country: "Scotland",
      },
      distillers: [500000],
    }),
  ).rejects.toThrowError(/Could not identify distiller/);
});

Typical use works as you'd expect:

test("requires authentication", async () => {
  const caller = createCaller({ user: null });
  const err = await waitError(
    caller.bottleCreate({
      name: "Delicious Wood",
      brand: 1,
    }),
  );
  console.log(getConstructorName(err));
  console.log(getConstructorName(TRPCError));
  expect(err).toMatchInlineSnapshot(`[TRPCError: UNAUTHORIZED]`);
});

stdout | src/trpc/routes/bottleCreate.test.ts > requires authentication
TRPCError
TRPCError

Still agree its probably an upstream issue in Chai, just wasnt sure if it made sense to try to catch it at this level given the package seems to be intended to type test errors.

[43081j]

there's something we're missing still i'm sure

could you tell me what errorLike is here:

check-error/index.js

Line 92 in fd56c9e

if (constructorName === '') {

i.e. just breakpoint it or dump a console.log in the if

is it really TRPCError? or is something somehow overriding what we think it is

whatever it is, isn't a class/constructor function. TRPCError meanwhile is a class. so maybe its getting something else and thats the problem here

[dcramer]

{ errorLike: [Function (anonymous)] }
{ constructorName: '' }

I tried to pinpoint what dep (thus release/commit) was causing this, but I'm using chai indirectly via vitest, and the lockfile is no joke.

Happy to try to dig deeper into chai but when I tried to use the debugger I couldn't effectively step into anything useful, and not sure where else I'd drop into in the chai bundled code to diagnose how its getting passed these values.

(could also totally be vitest's fault, but I dont know how the relationship works between the two)

[43081j]

no worries

i think vitest actually bundles its dependencies too, so you'll be reading through some (possibly minified) JS bundle

basically what this comes down to is that toThrow has been called with something which isn't an Error and isn't a class which constructs Error instances

it may be that we should throw a nice error in chai in those cases at least

but the 'fix' here is that we figure out what errorLike actually is and why its not the expected type. getConstructorName expects one of those two types, so it correctly throws when its not.

if you can't get breakpoints working, you could try console.log(errorLike.toString()) and hope it logs the function source

latest chrome also automatically ignores node_modules in devtools FYI. so you might wanna go in the ignore list in the options panel of dev tools and turn that off

[dcramer]

() => {
  throw object;
}

I'm guessing something changed with how its managing promise rejections and its not actually bubbling up the correct statement like it used to.

This is my MVP reproduction removing at least my userland code:

test("_repro", async () => {
  const promise = new Promise((resolve, reject) => {
    reject(new Error("test"));
  });
  expect(promise).rejects.toThrowError(/test/);
});

[43081j]

import {test, expect} from 'vitest';

test('foo', async () => {
  const promise = new Promise((resolve, reject) => {
    reject(new Error("test"));
  });
  expect(promise).rejects.toThrowError(/test/);
});

this passes for me locally with vitest 😞

i'd be really curious where that throw function is coming from 🤔

whatever is injecting that is the thing causing this problem

[dcramer]

This is not overly fun to look at, but here's the diff w/ the lockfile changes. Maybe something stands out to you:

https://github.com/dcramer/peated/pull/153/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bb

For context, thats a monorepo, but the relevant package is "apps/server". I did try downgrading vitest to 0.4.3 (whatever previous version I had) and hit same issues though.

Also can confirm its not the error-cause injection either.

[43081j]

ok i've found it

vitest-mock-axios is the culprit

> npm ls vitest

├─┬ vitest-mock-axios@0.1.0
│ └── vitest@0.24.5
└── vitest@1.4.0

here you can see it comes with its own very old version of vitest

internally, that version of vitest registers its jest-like chai plugin on import as a side-effect:
https://github.com/vitest-dev/vitest/blob/fb93a7e419e34559038910ee698818d739e2e6fb/packages/vitest/src/integrations/chai/index.ts#L2

that import results in this line happening:
https://github.com/vitest-dev/vitest/blob/fb93a7e419e34559038910ee698818d739e2e6fb/packages/vitest/src/integrations/chai/setup.ts#L9

which ultimately results in vitest overwriting Throw with its own implementation which overwrites the object we're testing (the function we're expecting will throw):
https://github.com/vitest-dev/vitest/blob/fb93a7e419e34559038910ee698818d739e2e6fb/packages/vitest/src/integrations/chai/jest-expect.ts#L64-L67

however, because modern vitest is also running at the same time, it too will overwrite chai's implementation (with the same logic).

that results in this:

// MODERN VITEST EXECUTES THIS:

        const object = utils.flag(this, 'object'); // At this point, this `object` is our function which we expect will throw
        if (promise === 'rejects') { 
          // We overwrite `this.object` with a wrapper function which itself throws `object`
          utils.flag(this, 'object', () => {
            throw object;
          });
        }

// OLD VITEST THEN EXECUTES THIS:

        const object = utils.flag(this, 'object'); // At this point, this `object` is the wrapped function modern vitest created
        if (promise === 'rejects') { 
          // We overwrite `this.object` with a wrapper function which itself throws _THE OTHER WRAPPER FUNCTION_
          utils.flag(this, 'object', () => {
            throw object;
          });
        }

therefore, this line of code in chai then throws:
https://github.com/chaijs/chai/blob/91e58ed96dc9993b6389af854a32a87f849fbc85/lib/chai/core/assertions.js#L2717-L2718

as its basically trying to get the constructor name of this logic:

const func = () => {
  return () => {
    throw err;
  };
};

try {
  func();
} catch (err) {
  // err is _still_ a wrapper, the inner one
  // () => { throw err; }

  getConstructorName(err); // fails of course
}

there's two fixes i think:

the immediate one is that you try move off the axios vitest package so you only have one copy of vitest in your tree

we could also improve chai to check that caughtErr is actually "error like" before we try get the constructor name (and if it isn't, fall back to toString of it maybe)

vitest probably could check that it isn't running twice too...

[dcramer]

Oof. Thanks for helping debug. Doesnt look like they have issues enabled, and given my limited use of it I'll likely just reimpelment the basic request mocks.