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

Shouldn't DNS error code 'EADNAME' be 'EBADNAME'? #3050

Closed
maarten-t opened this issue Sep 24, 2015 · 11 comments
Closed

Shouldn't DNS error code 'EADNAME' be 'EBADNAME'? #3050

maarten-t opened this issue Sep 24, 2015 · 11 comments
Labels
dns Issues and PRs related to the dns subsystem.

Comments

@maarten-t
Copy link

See https://github.com/nodejs/node/blob/master/lib/dns.js#L333

exports.ADNAME = 'EADNAME';

This line defines the error code ADNAME as 'EADNAME'. Shouldn't it be BADNAME and 'EBADNAME' respectively?

exports.BADNAME = 'EBADNAME';
@ChALkeR
Copy link
Member

ChALkeR commented Sep 24, 2015

The doc mentions BADNAME: https://nodejs.org/api/dns.html#dns_error_codes

@ChALkeR ChALkeR added the dns Issues and PRs related to the dns subsystem. label Sep 24, 2015
@silverwind
Copy link
Contributor

Looks like an mistake in ecfe32e. This change would be a semver-major.

@silverwind
Copy link
Contributor

(Unless that error code isn't returned by the API)

@ChALkeR
Copy link
Member

ChALkeR commented Sep 24, 2015

@silverwind Adding exports.BADNAME would be a semver-minor, removing exports.ADNAME would be a semver-major in any case (even if not returned by the API). So I would prefer that to be two separate commits, one of which could be pulled in 4.x.

@silverwind
Copy link
Contributor

What are these exports actually used for exactly? They don't seem to be involved in actual error handling. They certainly are undocumented, so semver rules don't strictly apply.

@silverwind
Copy link
Contributor

cc: @piscisaureus who added them in 858f230

@ChALkeR
Copy link
Member

ChALkeR commented Sep 24, 2015

@silverwind
Copy link
Contributor

Oh, right, comparison. Well, I think a replacement here would qualify as a bugfix to match the documentation, so I think it'd be fine in a patch release actually.

@silverwind
Copy link
Contributor

Filed #3051.

@trevnorris
Copy link
Contributor

Agreed that if the documented result doesn't match how the code works then it's a bug fix.

silverwind added a commit to silverwind/node that referenced this issue Sep 27, 2015
This error code export was mistakingly introduced in a 2012 commit which
added more error codes. The correct export.BADNAME was added in
nodejs#3051.

Semver: Major
PR-URL: nodejs#3051
Fixes: nodejs#3050
silverwind added a commit that referenced this issue Sep 27, 2015
This error code export was mistakingly introduced in a 2012 commit which
added more error codes. The correct export.BADNAME was added in
#3051.

Semver: Major
PR-URL: #3051
Fixes: #3050
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@silverwind
Copy link
Contributor

Landed in two seperate commits. exports.BADNAME will be available next version. exports.ADNAME will be removed in 5.0.0.

silverwind added a commit that referenced this issue Sep 30, 2015
Adds the documented but missing DNS error exports.BADNAME. This export
has been there before but got lost in a 2012 commit that added more
error codes. #3076 will remove the
wrong error code exports.ADNAME.

PR-URL: #3051
Fixes: #3050
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
silverwind added a commit that referenced this issue Sep 30, 2015
This error code export was mistakingly introduced in a 2012 commit which
added more error codes. The correct export.BADNAME was added in
#3051.

Semver: Major
PR-URL: #3051
Fixes: #3050
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 14, 2016
This error code export was mistakingly introduced in a 2012 commit which
added more error codes. The correct export.BADNAME was added in
#3051.

Semver: Major
PR-URL: #3051
Fixes: #3050
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 19, 2016
This error code export was mistakingly introduced in a 2012 commit which
added more error codes. The correct export.BADNAME was added in
#3051.

Semver: Major
PR-URL: #3051
Fixes: #3050
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants