-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
errorLike constructor can create misleading error #51
Comments
Found this which is related vitest-dev/vitest#5285 |
im guessing the error being thrown isn't an instance of anything do you know what exactly a 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 |
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. |
it does seem like a bug but in chai itself rather than check-error, basically. when you call 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 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 |
fyi works fine on TRPCError, but injecting into error-like itself to grab the value:
This comes from the rejects test, both all thats happening is caller.bottleCreate() is throw'ing a TRPCError instance.
Typical use works as you'd expect:
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. |
there's something we're missing still i'm sure could you tell me what Line 92 in fd56c9e
i.e. just breakpoint it or dump a console.log in the is it really 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 |
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) |
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 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 if you can't get breakpoints working, you could try latest chrome also automatically ignores node_modules in devtools FYI. so you might wanna go in the |
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:
|
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 |
This is not overly fun to look at, but here's the diff w/ the lockfile changes. Maybe something stands out to you: 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. |
ok i've found it
> npm ls vitest
├─┬ [email protected]
│ └── [email protected]
└── [email protected] 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: that import results in this line happening: which ultimately results in vitest overwriting 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: 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 vitest probably could check that it isn't running twice too... |
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. |
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 typicalthrow 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:
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.
The text was updated successfully, but these errors were encountered: