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

DNS resolver cancel does not return ECANCELLED #14814

Closed
JorgenPhi opened this issue Aug 14, 2017 · 2 comments
Closed

DNS resolver cancel does not return ECANCELLED #14814

JorgenPhi opened this issue Aug 14, 2017 · 2 comments
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. dns Issues and PRs related to the dns subsystem.

Comments

@JorgenPhi
Copy link

JorgenPhi commented Aug 14, 2017

  • Version: v8.3.0
  • Platform: Windows 10 Pro 1703 x64
  • Subsystem: dns

Hey! I am testing out the resolver.cancel method, and I am getting back ENOTFOUND instead of ECANCELLED.

Here's my script:

const { Resolver } = require('dns');
const resolver = new Resolver();
resolver.setServers([
  '123.45.67.89' // Not a real DNS Server
]);


function reverseLookup(ip) {
	// Variable to test if we got our answer back in time
	var matchfound = false;
	resolver.reverse(ip,function(err,domains){
		if(err) {
			// View our error
			console.error(err);
		} else {
			var matchfound = true;
			// Found a match, or two.
			domains.forEach(function(domain){
				console.log(domain);
			});
		}
	});
	setTimeout(function() {
		if(matchfound === false) {
			console.error("Took too long to reverse IP. Cancelling lookup.")
			resolver.cancel();
		}
	}, 1); // Set to 1 for testing
}

reverseLookup('8.8.8.8'); // google-public-dns-a.google.com

Output:

PS C:\Users\Jorgen\Documents\app_backend\tools> node .\dns-reverse-lookup.js
Took too long to reverse IP. Cancelling lookup.
{ Error: getHostByAddr ENOTFOUND 8.8.8.8
    at errnoException (dns.js:53:10)
    at QueryReqWrap.onresolve [as oncomplete] (dns.js:239:19)
  code: 'ENOTFOUND',
  errno: 'ENOTFOUND',
  syscall: 'getHostByAddr',
  hostname: '8.8.8.8' }
@refack refack added dns Issues and PRs related to the dns subsystem. net Issues and PRs related to the net subsystem. labels Aug 14, 2017
@refack
Copy link
Contributor

refack commented Aug 14, 2017

/cc @addaleax

@mscdex mscdex removed the net Issues and PRs related to the net subsystem. label Aug 14, 2017
@addaleax addaleax added the cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. label Aug 14, 2017
addaleax added a commit to addaleax/c-ares that referenced this issue Aug 14, 2017
When `ares_cancel()` was invoked, `ares_gethostbyaddr()` and
`ares_gethostbyname()` queries would fail with `ENOTFOUND` instead
of `ECANCELLED`.

It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
but I would appreciate review of the correctness of this change.

Ref: nodejs/node#14814
addaleax added a commit to addaleax/c-ares that referenced this issue Aug 14, 2017
When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
but I would appreciate review of the correctness of this change.

Ref: nodejs/node#14814
addaleax added a commit to addaleax/c-ares that referenced this issue Aug 14, 2017
When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
but I would appreciate review of the correctness of this change.

Ref: nodejs/node#14814
@addaleax
Copy link
Member

Seems like an upstream bug to me; see c-ares/c-ares#138

bagder pushed a commit to c-ares/c-ares that referenced this issue Aug 24, 2017
When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
but I would appreciate review of the correctness of this change.

Ref: nodejs/node#14814

Closes #138
addaleax added a commit to addaleax/node that referenced this issue Aug 24, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: nodejs#14814

Fixes: nodejs#14814
addaleax added a commit to addaleax/node that referenced this issue Aug 24, 2017
jasnell pushed a commit that referenced this issue Aug 29, 2017
Ref: #14814

PR-URL: #15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ghost pushed a commit to ayojs/ayo that referenced this issue Aug 30, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: nodejs/node#14814

Fixes: nodejs/node#14814

PR-URL: nodejs/node#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ghost pushed a commit to ayojs/ayo that referenced this issue Aug 30, 2017
Ref: nodejs/node#14814

PR-URL: nodejs/node#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ghost pushed a commit to ayojs/ayo that referenced this issue Aug 30, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: nodejs/node#14814

Fixes: nodejs/node#14814

PR-URL: nodejs/node#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ghost pushed a commit to ayojs/ayo that referenced this issue Aug 30, 2017
Ref: nodejs/node#14814

PR-URL: nodejs/node#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
cjihrig pushed a commit to cjihrig/node that referenced this issue Aug 31, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: nodejs#14814

Fixes: nodejs#14814

PR-URL: nodejs#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
cjihrig pushed a commit to cjihrig/node that referenced this issue Aug 31, 2017
Ref: nodejs#14814

PR-URL: nodejs#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: #14814

Fixes: #14814

PR-URL: #15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
Ref: #14814

PR-URL: #15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: #14814

Fixes: #14814

PR-URL: #15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
Ref: #14814

PR-URL: #15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
rvagg pushed a commit to rvagg/io.js that referenced this issue Sep 13, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: nodejs#14814

Fixes: nodejs#14814

PR-URL: nodejs#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
BridgeAR pushed a commit that referenced this issue Oct 2, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: #14814

Fixes: #14814

PR-URL: #15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
addaleax added a commit to addaleax/ayo that referenced this issue Oct 4, 2017
Original commit message:

  gethostbyaddr: fail with `ECANCELLED` for `ares_cancel()`

  When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
  queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

  It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
  but I would appreciate review of the correctness of this change.

  Ref: nodejs/node#14814

Fixes: nodejs/node#14814

PR-URL: nodejs/node#15023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
DronRathore pushed a commit to DronRathore/c-ares that referenced this issue Mar 11, 2020
When `ares_cancel()` was invoked, `ares_gethostbyaddr()`
queries would fail with `ENOTFOUND` instead of `ECANCELLED`.

It seems appropriate to treat `ares_cancel()` like `ares_destroy()`,
but I would appreciate review of the correctness of this change.

Ref: nodejs/node#14814

Closes c-ares#138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants