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

Set error codes on all errors #1711

Closed
1 task done
p-bakker opened this issue May 3, 2021 · 7 comments
Closed
1 task done

Set error codes on all errors #1711

p-bakker opened this issue May 3, 2021 · 7 comments
Labels
enhancement This change will extend Got features good for beginner This issue is easy to fix ✭ help wanted ✭

Comments

@p-bakker
Copy link

p-bakker commented May 3, 2021

What would you like to discuss?

On HTTPErrors the statusCode of the response is not exposed directly. You have to go through the HTTPError.response to get the statusCode

I would've expected the statusCode to be directly exposed on the HTTPError or otherwise set as the value of the code property, but neither is true

Is that as expected or is this a bug?

Checklist

  • I have read the documentation.
@szmarczak
Copy link
Collaborator

In regards to the documentation this is expected.

@p-bakker
Copy link
Author

p-bakker commented May 4, 2021

Tnx for the quick response.

I wasn't able to deduct this from the documentation, so thanks for clarifying

@p-bakker
Copy link
Author

p-bakker commented May 4, 2021

Additionally: shouldn't the code property on HTTPError instances be set to 'HTTPError'? According to the docs: Contains a code property with error class code, like ECONNREFUSED

@szmarczak szmarczak reopened this May 5, 2021
@szmarczak szmarczak changed the title HTTPError.code property not set to the statusCode of the HTTP Response property Set error codes on all errors May 5, 2021
@szmarczak szmarczak added enhancement This change will extend Got features good for beginner This issue is easy to fix ✭ help wanted ✭ labels May 5, 2021
@Drewfergusson
Copy link

Per @sindresorhus comments in #1739 it would be good to take a comprehensive approach to the code property for all error objects. The plain RequestError seems to always have a code associated with it that correlates to a Linux kind of error

[ 'ETIMEDOUT', 'ECONNRESET', 'EADDRINUSE', 'ECONNREFUSED', 'EPIPE', 'ENOTFOUND', 'ENETUNREACH', 'EAI_AGAIN' ]

The other child error messages all seem to return with the code property as undefined. As also stated it would be good to follow a Node.JS convention of ERR_*.

I might suggest

  • CacheError ERR_CACHE_ACCESS
  • ReadError ERR_READING_RESPONSE_STREAM
  • ParseError ERR_BODY_PARSE_FAILURE
  • UploadError ERR_READING_REQUEST_STREAM
  • HTTPError ERR_NON_2XX_3XX_RESPONSE
  • MaxRedirectsError ERR_MAX_REDIRECTS
  • UnsupportedProtocolError ERR_UNSUPPORTED_PROTOCOL
  • TimeoutError ERR_TIMEOUT
  • CancelError ERR_CANCEL_INVOKED

@sindresorhus
Copy link
Owner

CancelError ERR_CANCEL_INVOKED

ERR_CANCELED

@sindresorhus
Copy link
Owner

Otherwise, looks good to me.

@szmarczak ?

@szmarczak
Copy link
Collaborator

ERR_READING_REQUEST_STREAM -> ERR_UPLOAD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This change will extend Got features good for beginner This issue is easy to fix ✭ help wanted ✭
Projects
None yet
Development

No branches or pull requests

4 participants