auth-header icon indicating copy to clipboard operation
auth-header copied to clipboard

Add and export InvalidHeaderError

Open ricardogama opened this issue 7 years ago β€’ 10 comments

This PR adds the InvalidHeaderError which inherits from TypeError. Throwing a custom error and exporting it allows a more precise error handling, since it's possible to know that the error was thrown by parse. For example:

const { TokenExpiredError, verify } = require('jsonwebtoken');
const { InvalidHeaderError, parse } = require('auth-header');
const { UnauthorizedError } = require('src/errors');
const { userManager } = require('src/user');

function getUserByAuthorizationToken(header) {
  try {
    const { token } = parse(header);

    verify(token);

    return userManager.getUserByToken(token);
  } catch (e) {
    if (e instanceof InvalidHeaderError) {
      throw new UnauthorizedError('Invalid header');
    }

    if (e instanceof TokenExpiredError) {
      throw new UnauthorizedError('Expired token');
    }

    throw e;
  }
}

So I replaced all TypeError throws by InvalidHeaderError on the parse module and modified its tests accordingly, adding an assert for its message.

The InvalidHeaderError implementation was based on the Custom Error Types section of the MDN Error documentation.

ricardogama avatar Dec 05 '16 11:12 ricardogama

@izaakschroeder Any thoughts?

ricardogama avatar Dec 06 '16 11:12 ricardogama

Thinking of maybe using https://www.npmjs.com/package/es6-error

izaakschroeder avatar Dec 12 '16 17:12 izaakschroeder

Thanks for the contribution so far πŸ˜„ Will look at this in more detail tonight after I get some rest.

izaakschroeder avatar Dec 12 '16 17:12 izaakschroeder

@izaakschroeder If you're ok to add it as dependency that's awesome, just let me know.

BTW, any reason for babel-core being a dependency? Seems duplicated.

ricardogama avatar Dec 12 '16 17:12 ricardogama

@izaakschroeder ping

ricardogama avatar Dec 19 '16 10:12 ricardogama

Sorry for the delay, super busy as of late. Feel free to add the dependency πŸ˜„ And babel-cli should be a devDependency for generating dist, but that's it I think. If it's not like that, feel free to fix it up! πŸŽ‰ (Or I can get to it soonβ„’)

izaakschroeder avatar Dec 21 '16 12:12 izaakschroeder

@izaakschroeder Refactored in favour of es6-error and removed babel-core as dependency, please review.

ricardogama avatar Dec 21 '16 14:12 ricardogama

@izaakschroeder ping

ricardogama avatar Jan 13 '17 09:01 ricardogama

@izaakschroeder ping

ricardogama avatar Mar 01 '17 14:03 ricardogama

@izaakschroeder ping?

ricardogama avatar Apr 11 '17 13:04 ricardogama