`yieldsAsync` doesn't appear to wait for async callback to resolve

[markcellus]

Describe the bug I may be misunderstanding how yieldsAsync works. And if I am, I apologize. :see_no_evil: But it doesn't appear to wait until async callback finishes. See code snippet below.

To Reproduce Steps to reproduce the behavior:

        it('verifies yieldsAsync works', async () => {
            let works = false;
            const func = stub().yieldsAsync();
            async function check() {
                await func(async () => {
                    await new Promise((resolve) => setTimeout(resolve, 100));
                    works = true;
                });
            }
            await check();
            expect(works).to.equal(true);
        });

Expected behavior I'd expect to be true. After setting a debugger statement, I see that its false because the expect gets executed before the callback provided to func has a chance to resolve.

Screenshots If applicable, add screenshots to help explain your problem.

Context (please complete the following information):

• Library version: 15.0.0 (latest)
• Environment: Ubuntu 22.04
• Example URL:
• Other libraries you are using: N/A

Additional context Add any other context about the problem here.

[fatso83]

You are missing something :) Check the docs and the runnable example there. This functionality predates promises and the async syntactic sugaring. It is just about when to call the callback that is passed: stub.yields does a a synchronous call, while stub.yieldsAsync does so after the event loop has finished (think of it as being wrapped in setTimeout).

[fatso83]

To get some more guidance on achieving what you want, please post it to StackOverflow and tag it with sinon, so the bigger community can help answer your questions. We do not mind if you also post the link to the question here, btw 😄

[markcellus]

I've checked the docs, but the explanation and examples weren't as intuitive to my issue, so that's why I opened a ticket here.

This functionality predates promises and the async syntactic sugaring.

This is what I didn't know and what the docs don't necessarily point out. Are you suggesting that because this predates async that it isn't supported when using?

You are missing something :) Check the docs and the runnable example there.

This link takes me to fakes docs, which don't apply to my use case, so I didn't look there. But even after looking, it still doesn't seem to address the issue for my use case. Instead of just saying I'm missing something, can you explain what needs to be changed in my example in order to get it to work as you've intended?

[fatso83]

Instead of just saying I'm missing something, can you explain what needs to be changed in my example in order to get it to work as you've intended?

There are two things here:

1. I do not understand what you are trying to achieve with the test (apart from testing that it does something it does not do)
2. We are trying to keep the GitHub issues list tidy and focused on bugs and feature discussions. This ticket looks like a usage question, which is why I wrote it would be a better fit onStackOverflow. You probably would have your answer by now, as people are itching to up their rep on stuff like this 😄

The test itself says it tries to test yieldsAsync, which you are not using in any way intended. If you instead wrote the test in a way made it clearer what you were trying to achieve with fictional example, it would have been easier to guide you in the right direction (suggesting which functionality to use instead).

fakes docs, which don't apply to my use case

I am sorry for being too brief here. The thing is that it does apply to your use case of understanding what yieldsAsync does. Fakes exposes most of the same API stubs do. They actually rely on the same logic, but is a less troublesome version of the stubs functionality by having their behavior be immutable. So the thing I left out is that Fake#yieldsAsync and Stub#yieldsAsync is basically the same. But since if you check the Stubs docs for asynchronous calls you will see that there is no yieldsAsync example. Since there was one for fakes, I linked to that.

This is what I didn't know and what the docs don't necessarily point out. Are you suggesting that because this predates async that it isn't supported when using?

No. Asynchronous calls is of course nothing new. So we needed a way of supporting stubbing out such functionality. Say you had something like this old-skool Node code code. read-my-file.js

const fs = require('fs');

module.exports = function readMyFile(cb) {
    function fileHandler(error, buffer) {
        console.log('Finished reading buffer');
        cb(error, buffer);
    }

    fs.readFile('myfile.txt', fileHandler);
    console.log('Started reading file into buffer');
}

This would print "Started reading file into buffer" followed by "Finished reading buffer", as readFile is invoking the callback asynchronously. If you wanted to control the input to fileHandler in a test, one could started out with something like this

// some imports
...
const fs = require('fs');
const readMyFile = require('./read-my-file')

test("read-my-file passes the file content back into the callback", (done) => {
     // Arrange
     const stub = sinon.stub(fs, 'readFile');
     stub.yield(null, new Buffer("some file content"));

    // Act
    readMyFile( (err, buffer) => {
        // thankfully we don't need this try/catch with Promise based APIs :-)
        try {
            assert.equals(buffer.toString(), "some file content");
            done();
        } catch(error) {
            done(error);
        }
    });
});

The only problem with that test is that if you run it you will see it handles differently than its real counterpart, now printing out the console messages in the wrong order:

Finished reading buffer
Started reading file into buffer

This is of course caused by stub#yields being synchronous, immediately calling the callback. To make it behave like its real life version, we need to wait for the event loop to finish processing its current task, before calling the callback. That is the purpose of yieldAsync, so dropping it into the test above solves this.

So as you see, this bears very little relevance to the async syntax, which is only syntactic sugaring to be able to write this:

doSomeAsyncWork()
   .then(doSomethingElse)
   .catch(handleError)

into this

try { 
    const res = await doSomeAsyncWork()
    await doSomethingElse(res);
} catch (error) {
    handleError(error);
}

because this predates async that it isn't supported when using?

So the answer to this is that it is fully supported, but it does not make sense to use it like you do.

When you do this:

await func(async () => { // more code follows

you are implicitly saying that you expect func to return a Promise that can be awaited. As you see above, no such thing is happening. func will simply call the callback passed in asynchronously. It does not return Promises.

If you want to work with promises, I would look into using something like stub.resolves(value) (or even better, sinon.fake.resolves(value) 😉) which results in a stub that will return a resolved Promise, and combine that with some other functionality.

[fatso83]

After spending an hour on this, I think I am seeing that the functionality you want is for yieldsAsync to return a Promise in the cases where the callback that is passed in also returns a promise. So this would be possible:

/* could also be written like this: async function systemUnderTest(a,b,c){ await a+b+c; } */
function systemUnderTest(a,b,c) {
     returns Promise.resolve(a+b+c);
};

const fake = sinon.fake.yieldsAsync(1,2,3);
const result = await fake(systemUnderTest );
assert.equals(result, 6);

Right? It's not the most common use case, but essentially, that should be a non-breaking change AFAIK, so if you want that behavior, it should be a pretty minimal change. It would mean that yieldsAsync would then always return a Promise. To be consistent, that behavior should probably be present in other yield* methods as well, but it might be that this is all solved with the same change.

Care to supply a PR?

[markcellus]

the functionality you want is for yieldsAsync to return a Promise in the cases where the callback that is passed in also returns a promise

Yes. The asynchronous calls section of the docs says yieldsAsync (and all other async functions) are

same as their corresponding non-Async counterparts, but with callback being deferred at called after all instructions in the current call stack are processed

So based upon that, I thought "deferred" also meant waiting until the callback's promise resolved. But like you say earlier, it just means that it waits for setTimeout(callback, 0) or process.nextTick. So it's a misunderstanding on my part.

It's not the most common use case

That's odd. I would think that passing in an async function as a callback and waiting for it to resolve would be a more common use case and the need to defer the setTimeout or process.nextTick to be less common.

Either way, I've decided to just not use sinon for this case because it's clear that it's not able to handle it for now. I'll switch back over to yieldsAsync if the behavior I need ever gets implemented.

Thanks for the information.

[fatso83]

Given your requirements, it should not be the hardest thing to implement in Sinon, but you can at least get away with this yields implementation (only difference is that it returns the callback's return value) for now:

import referee from "@sinonjs/referee";

const { assert } = referee;

function yields(...args) {
    function stub(cb) {
        stub.called = true;
        stub.callCount++;
        return cb(...args);
    }
    stub.called = false;
    stub.callCount = 0;
    return stub;
}

it("should await the yields", async function () {
    // @returns resolved Promise
    async function systemUnderTest(a, b, c) {
        console.log("systemUnderTest", a, b, c);
        return a + b + c;
    }

    const stub = yields(1, 2, 3);
    const result = await stub(systemUnderTest);
    assert.equals(result, 6);
    assert(stub.called);
});