Skip to content

typescript-node: Reject Promises in using Error instances instead of plain objects #3872

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

Closed
ghost opened this issue Sep 11, 2019 · 7 comments · Fixed by #3876
Closed

typescript-node: Reject Promises in using Error instances instead of plain objects #3872

ghost opened this issue Sep 11, 2019 · 7 comments · Fixed by #3876

Comments

@ghost
Copy link

ghost commented Sep 11, 2019

Is your feature request related to a problem? Please describe.

In generated typescript code promises are rejected with plain objects instead of error instances.
By not using errors we loose stacktraces and thus easy debugging.

Describe the solution you'd like

Reject Promises with proper Error instances. This would be done here:

I can provide a PR, if you are going to accept it. This would be a breaking change.
I would add a new HttpError class with a statusCode property. Additionally We can add body and response. This would be a breaking change, but in my opinion a good one.

Describe alternatives you've considered

I don't think there is any viable alternative

Additional context

I come from kubernetes-client/javascript#336

@ghost ghost added the Enhancement: Feature label Sep 11, 2019
@ghost ghost changed the title [REQ] Feature Request Description [REQ] Reject Promises in typescript using Error instances instead of plain objects Sep 11, 2019
@macjohnny
Copy link
Member

if your proposed new HttpError class contains the properties body and response, it isn't a breaking change and thus could be included soon. so go ahead and file a PR.

@macjohnny macjohnny changed the title [REQ] Reject Promises in typescript using Error instances instead of plain objects typescript-node: Reject Promises in typescript using Error instances instead of plain objects Sep 11, 2019
@macjohnny macjohnny changed the title typescript-node: Reject Promises in typescript using Error instances instead of plain objects typescript-node: Reject Promises in using Error instances instead of plain objects Sep 11, 2019
@macjohnny macjohnny added this to the 4.1.3 milestone Sep 11, 2019
macjohnny pushed a commit that referenced this issue Sep 12, 2019
* Use HttpError class when rejecting promises

Fixes #3872

* Update samples

* Test the new code in client.ts
@keesvanlierop
Copy link

@macjohnny @aeb-sia If I'm not mistaken, the suggested improvement was pushed to 4.1.3 before making sure that body and response were included.

Without it, you cannot get any error context anymore about a failed request in your implementation. As you've suggested, we need to add those two properties to the HttpError class.

@macjohnny
Copy link
Member

@keesvanlierop feel free to file a PR if you suggest a change

@ghost
Copy link
Author

ghost commented Nov 14, 2019

@keenerthanyou actually dc2907a includes body, response and even statusCode

@keesvanlierop
Copy link

It accepts body and response in the constructor of HttpError, it doesn't actually do anything with it, right?

@ghost
Copy link
Author

ghost commented Nov 14, 2019

they are all using public in the constructor, so you can access them

@keesvanlierop
Copy link

You are right!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants