Error handling

[zeg-io]

What is the preferred method for handling jwt-simple errors?

I'd rather not just allow 500 errors to be returned by default, especially for stuff that has to do with expired tokens and the like.

It seems as though the internal code is just throwing Errors for everything. I'd also rather not try-catch the errors if possible...

Maybe I've misunderstood something?

[alexjab]

Hi,

all the errors thrown when decoding are due to invalid or expired tokens being passed by the user. So you can handle those by returning an error 401.

On the other hand, all the errors thrown when encoding a token, are due to you, the developer. These you should handle as error 500, because they should not happen, and let your error reporter warn you.

A good pattern:

• catch any error from jwt-simple,
• then throw an error with a custom error code using http-errors or boom,
• and then return the appropriate JSON to your user with the help of an error handler middleware.

Hope it gives you more info,

BR,

Alex

[zeg-io]

@alexjab Thank you for the considered response. Perhaps I was less than clear.

I understand how to return errors and that, in fact, the decoding should be returning 401's.

It seems to me based on the internal jwt.decode function that it is just throwing errors as message strings rather than providing reasons or a status.

For example:

• if there is an actual unhandled error, I'd expect an error to be thrown as it is now.
• if there is an issue with the client jwt, maybe return an object { valid: false, message: 'Expired.' }
• if the jwt is good, { valid: true, payload: {...} }

As it is now it seems I am left with Try ... Catch and then a parsing of the message to determine whether it's a 401 or an actual 500.

Does that make sense?

[alexjab]

Hi,

thanks for the clarification.

This library has a quite decent test suite, and I assume that your code does too. The former more or less guarantees that the library can not have an unexpected behaviour, and the latest that your implementation is correct.

On the other hand, what I wrote is usually easier said than done, and errors might happen regardless. One way to ensure than no error goes uncaught, is to use a switch to check for all the possible error messages that jwt-simple throws, and throw a 500 if it is unexpected. That might be a bit tedious, but this is not something that would be spread at several places in your codebase so it's not such a big deal, but it's not the most robust way to do it either.

An other possibility would be to throw custom-made errors for the cases that the library handles. A possibility would be to add a flag like isJwtError, like Joi does (https://github.com/hapijs/joi/blob/539760b773a0cade06e57d5af4d83715e4308f92/lib/errors.js#L181-L182).

@zeg-io does that sound fair to you ?
@hokaccha what do you think about this latter solution ?

Alex

[zeg-io]

@alexjab thanks for the clarification.

I don't claim to be super knowledgeable but it is my understanding at a high level that basically we (developers) want to have all failures bubble up to a higher level.

I don't consider correctly identifying an expired jwt to be a failure, nor even an error, just a different state of a successful operation if that makes sense.

I have further been told, and I'm not sure if it's accurate or not, but the dev that told me was very senior and speaks at node conferences, that I should avoid try/catch as it prevents operations from being cached.

I think the proposal you suggest (a property which distinguishes gets me halfway there)...

err.isJwtError provides a flag to check on, so I like that.

If there were a way to catch and handle an error prior to express grabbing it as a 500, and without using try-catch that'd be the other piece.

I don't know how one would accomplish that short of not throwing an actual error, but returning an object, which would break backwards compatibility.

[alexjab]

Hum I have never heard of issues regarding try...catch, but if you can find sources or articles that explain this in greater detail I would definitely be interested in knowing more about the reasons behind this.

A possible solution would be (again, like Joi does) to be have two methods, one that returns an error, and one (as it is currently done) that throws. If you need something like this, I'm afraid you would be better off finding one module that does it already (or fork this module), since trying to achieve this here would take quite some time of coding and reviewing (you might need this feature asap, and both the maintainer and I are on the other side of the globe).

Honestly, we have pushed quite some code in production using the throw pattern (at the company I work for), and we're quiet satisfied with how it works: basically, it enables us to defer and centralise error handling (we log the error, report it to sentry, and send a response to our users, all done in the error handler) in our web services. Returning an error instead of throwing it makes doing something like this much more difficult.

Actually, Joyent has an interesting article about error handling in node, and they seem to make a good case for the try...catch pattern when dealing with synchronous code and user input validation (in a sense, token validation is a user input validation, IMHO).

BR,

Alex

[zeg-io]

https://github.com/petkaantonov/bluebird/wiki/Optimization-killers

Looks like they have since fixed the issue and my source didn't realize it :)

[miguelaeh]

I had the same problem and I created a module based in jwt-simple that allow you to use the instanceof operator to know the type of the error.
https://www.npmjs.com/package/jwt-simple-error-identify

[satishsojitra]

I added like this:

 `  try{
        var decoded = jwt.decode(token, secret);           
    }catch(error){
        console.log(error.message);
        if(error.message == "Signature verification failed") resolve(false);
    }       ` 

If you change secret value or token value then it's throwing Signature verification failed error.

[M-erb]

I believe I maybe running into a similar issue. Correct me if I am wrong and I am happy to fix it or report somewhere else. The encode and decode seem work work perfectly, until that is I change the returned token and try to decode it.

Below I have wrapped the decode in a promise with a try catch. Again when I enter a proper token this succeeds with no issues.

function decode (token) {
  return new Promise((resolve, reject) => {
    try {
      let decoded = jwt.decode(token, secret)
      resolve(decoded)
    } catch (error) {
      console.log('decode error: ', error)
      reject(false)
    }
  })
}

If I for example take that same token but add a "9" near the beginning of the token string then run it through this decode function I get a console.log error of "decode error: SyntaxError: Unexpected end of JSON input"

What I assume is happening is the method used to parse the JSON from the token is not currently designed to handle non-JSON strings, maybe? not sure. I am happy to help if any way I can, including to shut up about it if it is only happening to me =D

[msageryd]

I'm using JWT as information carrier for email validation in my app. I would really like to give the user some feedback if the token has expired. As jwt-simple handles expired tokens as an error, I'm thinking of putting the expiration date in a private claim in the token.

This way I'll be able to decode the token without errors and then manually checking the expiration date. Not the nicest solution, I know. But to me it's nicer than relying on a string in the thrown error.

Any thoughts on this?

[miguelaeh]

Sorry for the late. Some time ago I had the same problem. I modified the source code of jwt-simple to identify the error type at the decode. You can see it at https://www.npmjs.com/package/jwt-simple-error-identify