Repository: node-nock/nock
Pull request #898 (2017-05-11 20:17:47)

Add an error code to NetConnectNotAllowedError
This is in line with [nodejs error codes](https://nodejs.org/api/all.html#errors_error_code
). I chose `ENETUNREACH` because it clearly defines the reason why the http request failed.

This helps testing when using an http request library that does retries as this isn't an error you would normally retry.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/11483209/badge)](https://coveralls.io/builds/11483209)

Coverage increased (+0.005%) to 92.691% when pulling **27112ae462e7337e6dd0b0a48894d430581dee04 on reconbot:error-code** into **17d7c25682ea520b0a255e933b64d5c7f355330f on node-nock:master**.

paulmelnikow: As I articulated in #884, NetConnectNotAllowedError is a programmer error, not a runtime error. Nock should treat it _less_ like a network error, not more. I would argue against this change.
paulmelnikow: To say this another way:

- To simulate a network error and trigger your application's error handling code, use `.replyWithError({ code: 'ENETUNREACH' })`.
- To assert that a test does not make a network request, use `.disableNetConnect()`.
reconbot: I'm using tools that retry on recoverable errors but not others. ([`got`](https://www.npmjs.com/package/got)) There's no standard error that says network is disabled, `ENETUNREACH` was the closest error code from the standard http error set that could be handled properly by downstream libraries.
gr2m: @reconbot can you pass a function to the retry option that checks for the `NetConnectNotAllowedError` error name and use that as a way to not retry the request?
reconbot: It's feels more weird to introduce nock to my production code than it does
to simulate a network error when I want to simulate a network error.

On Fri, Sep 22, 2017, 8:35 PM Gregor Martynus <notifications@github.com>
wrote:

> @reconbot <https://github.com/reconbot> can you pass a function to the
> retry option that checks for the NetConnectNotAllowedError error name and
> use that as a way to not retry the request?
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/node-nock/nock/pull/898#issuecomment-331590038>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AABlbloLsAu4EAykGvA3sGfqwMtKXvHAks5slFJugaJpZM4NYgE->
> .
>

gr2m: I don’t understand what you mean Francis :) Is this PR still relevant for you or can we close it? And if it is, can you explain in more detail?
reconbot: Say I have an application function

```js
import got from 'got'
import {authHeaders} from './config'

export default function (url) {
   return got(url, { authHeaders, retries: 5 })
}
```

And I don't properly mock this request in my tests. [`got`](https://www.npmjs.com/package/got) will retry the request 5 times and probably timeout my test. Got and my app have no knowledge of `nock` or any of the testing tools. It does however know about the standard errors returned by [`nodejs#errors`](https://nodejs.org/api/errors.html#errors_node_js_error_codes). It's able to automatically not retry unrecoverable errors. This isn't unique to `got`, the `aws-sdk` has a similar behavior.

I don't want to import `nock` into my production deps to check to see if it's an instance of the  `NetConnectNotAllowedError`.

I'm proposing we use a standard error code that suggests the requests shouldn't be retried when preventing a network connection. The specific code isn't important to me but I know `ENETUNREACH` is widely supported. It also clearly states the outcome of a disabled net connection. It is as if you really disabled your network connection.
gr2m: I’m okay merging it. Could you rebase your PR on the latest master?
coveralls: 
[![Coverage Status](https://coveralls.io/builds/13419055/badge)](https://coveralls.io/builds/13419055)

Coverage increased (+0.005%) to 92.691% when pulling **088e31529327c7928de141e01c4f28d2443eaea9 on reconbot:error-code** into **66cd477dbc3c262b4f55d045f161f0de3a2a0b15 on node-nock:master**.


Patch:
diff --git a/lib/intercept.js b/lib/intercept.js
index 9ed49af..41ff8b5 100644
--- a/lib/intercept.js
+++ b/lib/intercept.js
@@ -31,6 +31,7 @@ function NetConnectNotAllowedError(host, path) {
   Error.call(this);
 
   this.name    = 'NetConnectNotAllowedError';
+  this.code    = 'ENETUNREACH'
   this.message = 'Nock: Not allow net connect for "' + host + path + '"';
 
   Error.captureStackTrace(this, this.constructor);


Test patch:
diff --git a/tests/test_intercept.js b/tests/test_intercept.js
index 27340b0..54c204c 100644
--- a/tests/test_intercept.js
+++ b/tests/test_intercept.js
@@ -2833,12 +2833,13 @@ test('NetConnectNotAllowedError is instance of Error', function(t) {
   nock.enableNetConnect();
 });
 
-test('NetConnectNotAllowedError exposes the stack', function(t) {
+test('NetConnectNotAllowedError exposes the stack and has a code', function(t) {
   nock.disableNetConnect();
 
   http.get('http://www.amazon.com', function(res) {
     throw "should not request this";
   }).on('error', function (err) {
+    t.equal(err.code, 'ENETUNREACH')
     t.notEqual(err.stack, undefined);
     t.end();
   });