Incorrect custom async matcher error message

[mattphillips]

🐛 Bug Report

Custom async matchers are throwing with an error message prefixed with Error and not showing the source code of where the error originated from.

To Reproduce

Given the example in the docs, I get the following output:

expect.extend({
  async toBeDivisibleByExternalValue(received) {
    const externalValue = await Promise.resolve(1000);
    const pass = received % externalValue == 0;
    if (pass) {
      return {
        message: () =>
          `expected ${received} not to be divisible by ${externalValue}`,
        pass: true,
      };
    } else {
      return {
        message: () =>
          `expected ${received} to be divisible by ${externalValue}`,
        pass: false,
      };
    }
  },
});

test('is divisible by external value', async () => {
  await expect(100).toBeDivisibleByExternalValue();
  await expect(101).not.toBeDivisibleByExternalValue();
});

Expected behaviour

A pointer to the source and the message highlighted correctly.

[SimenB]

I was 100% sure this worked! :o
Will look into it tomorrow (or happy to review if you're working on it)

[mattphillips]

I took a look into it but couldn't see where the callsite was being set wrong 🤷‍♂️ any ideas? btw PR incoming with a refactor of the captureStackTrace code ;)

[SimenB]

We should be setting up an error in all assertions before calling its implementation for use in async traces. Somewhere in expect, but I'm currently on mobile, so hard to dig up

[mattphillips]

Yup thats where I checked but it all looked good to me - might need your debugging skills on it :)

[SimenB]

I'll try tomorrow.

If you think errors can be documented/made clearer I'd be happy to do so! It's really quite straightforward, so I feel any confusion on the traces is a failure on my part in writing grokkable code...

[rickhanlonii]

@SimenB did you have a chance to look at this?

[gziolo]

I'm experiencing the same error at the moment. As long as the matcher is sync, everything works as expected. As soon as I mark it as async and use await expect I see it printed on the console the same way as @mattphillips shared. You don't even need to use any async code in the matcher to experience that.

[theotherdon]

@gziolo I was experiencing the same thing. I'm pretty sure I figured out the issue though. I'm hoping to open a PR with the fix a little bit later today.

[SimenB]

Completely forgot about this. It's even worse in Jest 24 actually 🙀

[SimenB]

@DoNalD-S oh, that's awesome! I won't dig into it then. Looking forward to your PR! 🙂

[theotherdon]

Great! 🙂 I did manage to figure out how to fix the problem. I'm just trying to get the initial test run to pass before I dive in to start implementing my own test.

The problem is due to us losing the context of the current stack when we create the error here:https://github.com/facebook/jest/blob/6cc2a85621cac07387e82811a6cbe1222c3f449b/packages/expect/src/index.js#L263 The error is being created after the promise resolves here, by which point the previous stack has been discarded: https://github.com/facebook/jest/blob/6cc2a85621cac07387e82811a6cbe1222c3f449b/packages/expect/src/index.js#L307

I'm going to try to try to get the tests passing on my machine in the morning and hopefully open a PR.

[SimenB]

@DoNalD-S were you able to make it work?

[theotherdon]

@SimenB Yes, I was able to make it work. I just have a couple of failing snapshots to update. I'm hoping to have the PR today or tomorrow.

[theotherdon]

@SimenB I have the issue fixed (here's the commit, if you'd like to see it), but there are a couple of other tests that are failing due to an extra line being added to the stack trace that I'm having a hard time debugging. Do you happen to know where the failure messages are being built? I've been digging around for a while and haven't quite been able to pin it down. If you're able to point me in the right direction, I'm pretty sure I can get this wrapped up shortly.

[SimenB]

@DoNalD-S hey! feel free to open up a PR, then we can comment on it (and see the test run result on CI) 🙂

(I'd recommend to keep the refactoring to a minimum for a bug fix, and rather submit a separate PR for organizational refactoring (they look good!))

[theotherdon]

@SimenB Thanks for the recommendation! 🙂 I just opened PR #7652 and also kept the refactoring to a minimum. (I'll open a separate PR for the refactor once we merge this one.) I left a question for you regarding removing a couple of lines from the stack trace.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.