node-oauth icon indicating copy to clipboard operation
node-oauth copied to clipboard

Create a real error object on bad HTTP response

Open evanp opened this issue 10 years ago • 6 comments

I frequently find myself special-casing the OAuth request error responses, since they are not instances of the Error class.

This change adds a new class, OAuth.Error, and returns it in case of errors in the HTTP requests. It has the same properties as the original Object instance.

evanp avatar Aug 05 '13 18:08 evanp

Yeah, I have very few regrets generally in life, but not passing back a real error was a definite mistake, I can't even remember why I chose to do that at the time (I do know better, honest)

I guess my only issue would be whether changing it could break dependant behaviour, if someone is depending on 'Error' vs 'Object literal' to determine whether it is a failure in the OAuth library vs a failure in the underlying request (but since both cases are likely equally fatal, I guess the continuation would be the same in eitheer case?)

ciaranj avatar Aug 05 '13 18:08 ciaranj

+1

jonathanong avatar Sep 01 '13 09:09 jonathanong

:+1: I wouldn't worry too much about breaking people's theoretical code depending on errors not being Errors. Although since you're already at 0.9.12, you could publish 1.0.0 which would be very strictly semver compliant.

1j01 avatar May 05 '15 15:05 1j01

@ciaranj , you don't have to worry about breaking existing behavior if you are careful about your (semver) versioning (afterall, that is what semver is all about). I totally agree that returning actual Errors is something that should be implemented right away. It's a long-awaited feature that is getting ignored (#84, #250, #155).

If anything, the community will thank you for the breaking change if it means giving them something they want! 😄 👍

radiovisual avatar Apr 25 '16 14:04 radiovisual

@ciaranj , I wrote a module which I think is a workaround to this problem. Users of the current (and previous) releases of the oauth module can simply install my module via npm, so there is no messing with the current API or breaking changes in the oauth module.

The module I wrote is called node-oauth-error, which takes in the error objects returned from oauth and converts them into actual Error objects (complete with stack traces).

It's as simple as passing the oauth error object into node-oauth-error's constructor, like this:

new OAuthError(oauthErrorObject);

Here is a usage example from the module's readme:

const oauth = require('oauth');
const OAuthError = require('node-oauth-error');

oauth.get('some/url/endpoint',
    credentials.accessToken,
    credentials.accessTokenSecret,
    (err, data) => {
        if (err) {
            // convert the oauth error object into a real `Error()`.
            throw new OAuthError(err);
        }
        // ...
    }
);

Pull requests or suggestions are welcome if you want to take a closer look.

radiovisual avatar Apr 27 '16 12:04 radiovisual

+1

@ciaranj Do you have any plan to merge this pr?

popomore avatar Jun 27 '16 17:06 popomore