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

disableNetConnect() / "Not allow net connect" errors are swallowed up #884

Open
paulmelnikow opened this issue Apr 18, 2017 · 8 comments
Open
Assignees

Comments

@paulmelnikow
Copy link
Member

I'm testing the API for shields, which is a server that sits in front of a bunch of third-party APIs. I wrote a plugin that integrates Nock into an API testing framework. Overall it's been fun and is working well!

I did encounter a surprising behavior which was tricky to debug. When I have disabled network connections, if my application makes a request, I want my tests to fail with an error about the unexpected request.

The observed behavior is that the error is passed to the application like any other network error. It gets processed by the application's error-handling code, as if it were an ECONNREFUSED or ECONNRESET. In other words, Nock simulates the effect of network errors.

The expected behavior is that the error bubble up as an unhandled exception, to be caught by the test runner. By throwing an exception, Nock is saying, "you asked me not to allow unexpected network connections, but then you made one." You're on lockdown. Nock forbids unexpected connections.

I can get nearly the behavior I want by calling scope.done() after my tests, though the error message is opaque, and doesn't make it clear an unauthorized request was rejected.

Nock tests are often tricky to write, requiring trial and error. Until they are correct, they fail silently, because everything has to be perfect for the request to match. And if I'm doing TDD, I don't know if the test or the application is at fault. Throwing the error at the time of the unauthorized connection would make this more transparent.

This odd issue tends to come up because I'm using mostly live requests. I'm using mocks to cover the error-handling paths which are impossible to test with live requests, including the path that handles connection errors. When the error is swallowed up, it's particularly baffling.

I can see use cases, even in Shields, where a dev wants to simulate network errors, though the "lockdown" behavior would seem a clearer default.

The documentation reads:

The returned http.ClientRequest will emit an error event (or throw if you're not listening for it)

In practice, anyone using a wrapper like request will see an error, not a thrown exception.

@BenMGilman
Copy link

+1

@jasperkuperus
Copy link

jasperkuperus commented May 24, 2017

Sounds similar to what I experience. My app handles the errors thrown by nock, but I'd like my spec to fail. I would like to be able to ask nock if there were unmatched requests during the spec, similar to how you can ask if there are still pending mocks, e.g.:

nock.unmatchedRequests()

For now, I've solved it like this in my spec:

nock.emitter.on('no match', (request) => {
  throw Error('Request fired that did not match what was mocked', request);
});

It would be nice if this unmatchedRequests method would also consider nock.enableNetConnect('127.0.0.1');. Now I have to filter myself on the no match event.

@stale
Copy link

stale bot commented Sep 16, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.

@stale stale bot added the stale label Sep 16, 2018
@stale stale bot closed this as completed Sep 23, 2018
@lock
Copy link

lock bot commented Oct 7, 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 Oct 7, 2018
@paulmelnikow paulmelnikow reopened this Feb 3, 2019
@nock nock unlocked this conversation Feb 3, 2019
@stale stale bot closed this as completed Feb 10, 2019
@paulmelnikow paulmelnikow reopened this Feb 12, 2019
@stale stale bot removed the stale label Feb 12, 2019
@gr2m gr2m added the pinned label Feb 14, 2019
@mastermatt
Copy link
Member

@paulmelnikow should this issue remain open? It's unclear to me what the action item is.

@danon
Copy link

danon commented Aug 24, 2020

I think this would be most helpful

@mastermatt
Copy link
Member

@danon I think the ask here is still unclear, could you shed some light on how you would want this to work? Or what struggles you were dealing with that brought you to this ticket?

This issue seems, to me, to be with the various HTTP clients handling unexpected errors on requests differently. As a way around that, Nock provides an event-based mechanism to get at the errors instead.

@mikicho
Copy link
Contributor

mikicho commented Jun 26, 2021

@mastermatt see #2211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants