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

Missing Error Handler on aliasHandler #60

Closed
theRichu opened this issue Jul 10, 2018 · 11 comments
Closed

Missing Error Handler on aliasHandler #60

theRichu opened this issue Jul 10, 2018 · 11 comments
Labels

Comments

@theRichu
Copy link

Hi,

I encounter 'unhandled rejection' error when upgrade moleculer-web v0.6.3 to v.0.8.0

and I've found error handler is missing on aliasHandler,

In my situation,
in authorize function, return promise with rejection I've got unhandled rejection error

 return Promise.reject(new UnAuthorizedError(ERR_NO_TOKEN))

and my suggestion

src/index.js :392


        // If not, it handled request/response by itself. Break the flow.
       return true;
        })
        .catch(err => this.sendError(req, res, err));
},
@icebob icebob added the bug label Jul 10, 2018
@icebob
Copy link
Member

icebob commented Jul 10, 2018

Thanks, I'm fixing.

@icebob
Copy link
Member

icebob commented Jul 10, 2018

Could you show relevant parts of your code? I can't reproduce it. For me, no unhandled rejection, the error goes back to the caller properly.

Custom alias:

aliases: {
	"POST users": "users.create",
	"health": "$node.health",
	"custom"(req, res) {
		res.writeHead(201);
		res.end();
	}
},

with this authorize: https://github.com/moleculerjs/moleculer-web/blob/master/examples/full/index.js#L299

icebob added a commit that referenced this issue Jul 10, 2018
@theRichu
Copy link
Author

Not in alias,
rejection in authorize method,
I called action to service via broker

You may reproduce this way,

const {
  UnAuthorizedError
} = ApiGateway.Errors
authorize (ctx, route, req, res) {
    if (req.needAuth !== 'required') {
        return ctx
      }
      let token
      if (req.headers.authorization) {
        let type = req.headers.authorization.split(' ')[0]
        if (type === 'Token') {
          token = req.headers.authorization.split(' ')[1]
        }
      }
      if (req.needAuth === 'required' && !token) {
        return Promise.reject(new UnAuthorizedError(ERR_NO_TOKEN))
      }
      // Verify JWT token
      return ctx.call('users.resolveToken', {
        token
      })
        .then(user => {
          return Promise.reject(new UnAuthorizedError(ERR_NO_TOKEN))
        })
}

and request any route.

and get
Unhandled rejection TokenExpiredError: jwt expired

@icebob
Copy link
Member

icebob commented Jul 12, 2018

By the error message, I think the problem is not in authorize, but in users.resolveToken. Could you show the source of this action?

@theRichu
Copy link
Author

resolveToken: {
      cache: {
        keys: ['token'],
        ttl: 60 * 60 // 1 hour
      },
      params: {
        token: 'string'
      },
      handler (ctx) {
        return new this.Promise((resolve, reject) => {
          jwt.verify(ctx.params.token, this.settings.JWT_SECRET, (err, decoded) => {
            if (err) {
              return reject(err)
            }
            resolve(decoded)
          })
        }).then(decoded => {
          if (decoded.user) {
            return User.findOne({
              where: {
                id: decoded.user
              }
            }).then(user => {
              return user.dataValues
            })
          }
        })
      }
    },

This is my resolveToken code

@theRichu
Copy link
Author

I know the error message is from jwtwebtoken package,
the problem is I can't handle the rejection error anywhere.
So, request are just pending when token is expired or invalid.

I just want to response error code and message.

@icebob
Copy link
Member

icebob commented Jul 18, 2018

The return reject(err) is called when the token is expired or invalid?

@theRichu
Copy link
Author

Yes,

@icebob
Copy link
Member

icebob commented Jul 18, 2018

I tried it but no unhandled rejection. Could you create a small repro repo?

@theRichu
Copy link
Author

https://github.com/theRichu/moleculer-web-unhandled

check this repo please

@icebob
Copy link
Member

icebob commented Aug 1, 2018

Thanks. The unhandled issue is fixed in the master branch. You can update & test it with npm i moleculerjs/moleculer-web.
By the way, the calling order changed and there is a side effect in your repo. The onBeforeCall & authorize are called before action middlewares. So 'GET /user/me': [auth.required(), 'users.me'] won't work because the authorize called sooner than auth.required().

@icebob icebob closed this as completed Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants