Skip to content
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

reply with an asynchroneous response. #283

Merged
merged 2 commits into from Mar 16, 2015

Conversation

Natim
Copy link

@Natim Natim commented Mar 13, 2015

I am trying to do the following:

  function readAws() {
    return nock('https://s3.amazonaws.com')
      .filteringPath(function filter(_path) {
        id = _path.replace('/' + bucket + '/', '');
        return _path.replace(id, 'XXX');
      })
      .get(u)
      .reply(200, function() {
        console.log(arguments);
        var s = through();
        s.setEncoding = function() {};
        console.log("ID", id);
        local.read(id, function(err, data) {
          if (err) throw err;
          s.queue(encode(data));
          s.end();
        });
        return s;
      });
  }

The aim is to basically use the local file storage api to mock the remote aws one.
But by the time I am reading the buffer, it is still empty.

The good call would be to be able to get a callback on the reply callback function in order to tell nock when it is finished.

@pgte
Copy link
Member

pgte commented Mar 13, 2015

yeah, we don't support an asynchronous handler. I would love a PR with that :).

Basically nock would have to check function arguments length to see if the function would or not be asynchronous and provide a callback when it is.

@Natim
Copy link
Author

Natim commented Mar 13, 2015

Yes it seems that mocha is doing a similar thing with the done parameter.

@Natim
Copy link
Author

Natim commented Mar 13, 2015

The code to add stand apparently there: https://github.com/pgte/nock/blob/master/lib/request_overrider.js#L264-L268

@Natim
Copy link
Author

Natim commented Mar 13, 2015

@pgte You asked for a PR, here is my proposal.

r? @n1k0

responseBody = new Buffer(responseBody);
} else {
responseBody = JSON.stringify(responseBody);
function continueWithResponseBody(responseBody) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this line I didn't touch the code just indent it because of the new function definition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the callback should be using the standard callback pattern: error-first, so that it can be easily composed with other I/O.

This allows stuff like:

.reply(function(cb) {
  fs.readFile(file, cb);
});

or even better:

.reply(fs.readFile.bind(fs, file));

Then the answer comes: what happens when there is an error happens? I propose that it's just thrown so that the test stops.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes good point, I was thinking about it and then I realize that there should not be errors at that point but you've made your point, I will change that.

@Natim
Copy link
Author

Natim commented Mar 13, 2015

Updated with @pgte comments.

@pgte pgte merged commit b0c53f9 into nock:master Mar 16, 2015
@pgte
Copy link
Member

pgte commented Mar 16, 2015

@Natim thanks!

Landed on v1.2.0

@almet
Copy link

almet commented Mar 16, 2015

Awesome, thanks @pgte.

@lock
Copy link

lock bot commented Sep 14, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants