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

instanceof HttpError not working when receiving error from other package #56

Closed
petermorlion opened this issue Nov 14, 2018 · 7 comments
Assignees

Comments

@petermorlion
Copy link

I'm not sure if this is specific to http-errors or to TypeScript, but this is the issue I'm having: it seems that instanceof doesn't work when I'm receiving a HttpError from another package.

To reproduce:

  • create a package with npm init
  • npm install typescript http-errors --save
  • npm install @types/http-errors --save-dev
  • add index.ts:
import * as httpErrors from "http-errors";

export function getError() {
    return new httpErrors.Unauthorized("Whatever");
}

To have exactly the same as I did, run ./node_modules/typescript/bin/tsc --init and change target from es5 to es6.

Now run npm link in the directory and create a new project somewhere else and npm link <first-package-name> to add a symlink in the node_modules directory.

In the new project, add some code like this:

import * as httpErrors from "http-errors";
import { getError } from "<first-package-name>";

const err1 = new httpErrors.Unauthorized("Foo");
console.log(err1 instanceof httpErrors.HttpError); // will output true

const err2 = getError();
console.log(err2 instanceof httpErrors.HttpError); // will output false!

Again, I'm not sure if this is TypeScript in general, or the way the code in http-errors is contructed. I'm no JS/TS expert, so I'm interested in your opinion/advice.

For your convenience, I've created a repo that you can just pull and then run node index.js.

@dougwilson
Copy link
Contributor

Thanks for your report! It sounds like you may be describing this Node.js issue with how node_modules and instanceof interact: nodejs/node#17943

@petermorlion
Copy link
Author

Thanks for the quick answer! Yes, that would probably be an issue too. But this StackOverflow question makes me think it can even happen with the same versions. Just different instances of the module, due to how npm works.

I could solve this by checking the status property, i.e.:

if (err.status === 401) {}

But it gets a bit harder if I want to know if it's a HttpError or a generic Error.

So I will be looking into why npm isn't flattening the node_modules.

@petermorlion
Copy link
Author

Related: npm/npm#19770

I've found I can work around it by running npm dedupe, so it's not an issue with TypeScript, not with http-errors, but with npm. I guess.

Unfortunately, the npm team seems to have moved to Discourse for their issues. Well, that's not what's unfortunate, but the issue is now locked, without a solution, and I can't find it in their forum anywhere.

You can close this issue if you want. I'll be playing around with different npm versions (mine is 5.7.1) and see what I can find out.

@dougwilson
Copy link
Contributor

Hi @petermorlion thanks for the additional info. I usually don't close it out unless it's turned stale. You raise a valid issue, but I'm just not sure if it's possible for this module to solve, but open to ideas on if it can be and what the changes may look like 👍

@eugef
Copy link

eugef commented Dec 19, 2018

@dougwilson I have the same problem, cause i need to distinguish between generic Error and HttpError

So far i am also using status property, but it would be more obvious if HttpError has a name property with HttpError value.

Then the check can look like: if (error.name === 'HttpError') { ... }
This will work even if there are multiple instances of http-error package

What do you think?

@dougwilson
Copy link
Contributor

Hi @eugef yes, the errors do have a name you can check. The name will be reliable as long as you do not minify the code. The check would look like this: if (error.constructor && error.constructor.super_.name === 'HttpError') { ... }

@dougwilson
Copy link
Contributor

I know this is an old issue, but just wanted to comment if you're around: I've been working to clean through these issues, and this remained open because I felt like we should try and do something. Thinking back, perhaps we can provide an export isHttpError(val) that would return true for anything coming out of createError--this would include "duck typing" in order to account for the issue at hand here, where instanceof may not actually work.

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

No branches or pull requests

3 participants