Add an error code to NetConnectNotAllowedError

[reconbot]

This is in line with nodejs error codes. 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 increased (+0.005%) to 92.691% when pulling 27112ae on reconbot:error-code into 17d7c25 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) 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 ***@***.***> 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 <#898 (comment)>, 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

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 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. 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 increased (+0.005%) to 92.691% when pulling 088e315 on reconbot:error-code into 66cd477 on node-nock:master.

[lock]

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!