Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Throw HttpClientError instead of Error #32

Merged
merged 2 commits into from
Oct 12, 2020

Conversation

dhadka
Copy link
Member

@dhadka dhadka commented Aug 18, 2020

The <verb>Json helper methods throw an Error when the server responds with a non-zero status code. They also attach the status code and result as properties.

err['statusCode'] = ...
err['result'] = ...

This, however, is not compatible with TypeScript strict type checking. As a result, code depending on http-client that use strict type checking (such as actions/toolkit) can't read the status code and result properties without resorting to TypeScript hacks, such as casting the error object to any and ignoring the resulting lint warnings.

This PR adds a new error subclass, HttpClientError, which properties to store the status code and result. Throwing a different error type also lets callers distinguish between errors related to the API call from other generic errors, such as socket errors.

@dhadka
Copy link
Member Author

dhadka commented Aug 18, 2020

Tests failing because of postmanlabs/httpbin#617

super(message)
this.name = 'HttpClientError'
this.statusCode = statusCode
Object.setPrototypeOf(this, HttpClientError.prototype)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a short look into the pull request and was wondering what this line is about. Isn't it redundant? I thought the prototype of this (in a constructor) is <class>.prototype by default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Didn't know about this issue. Thanks for your reply.

@bryanmacfarlane
Copy link
Member

It looks like we need to re-open and merge #33 as a pre-req of this. correct @dhadka ?

@bryanmacfarlane bryanmacfarlane merged commit edadda1 into actions:main Oct 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants