Combination of Promise.reject() and throw new Error()

[tksugimoto]

Thank you for a wonderful library!

---

environment

• node-oauth2-server: v3.0.0
• node: v6.10.3

---

In the document (OAuth2Server — oauth2-server 3.0.0 documentation) , it says that exceptions are returned by Promise.reject(), but some are thrown.

A Promise that resolves to the authorization code object returned from Model#saveAuthorizationCode(). In case of an error, the promise rejects with one of the error types derived from OAuthError.

Therefore, we must use Promise.prototype.catch() and try-catch together.

An example

all codes

try {
	const request = generateRequest(clientId);
	oauth.authorize(request, response, options).then(() => {
		console.log(`clientId = ${clientId}, authorized`);
	}).catch((err) => {
		console.log(`clientId = ${clientId}, catch error in promise`, err);
	});
} catch (err) {
	console.log(`clientId = ${clientId}, catch error in try-catch`, err);
}

• If clientId is valid then authorized
• If clientId is invalid then catch error in promise { invalid_client: Invalid client: client credentials are invalid ...
• If clientId is null then catch error in try-catch { invalid_request: Missing parameter: client_id ...

Reason

• AuthorizeHandler.prototype.handle calls this.getClient before Promise chain start.
  • node-oauth2-server/authorize-handler.js at v3.0.0 · oauthjs/node-oauth2-server#L72-L89
• AuthorizeHandler.prototype.getClient throw Error if client_id parameter is missing.
  • Since it is not Promise.reject(), the exception will be transmitted to the outside.
  • node-oauth2-server/authorize-handler.js at v3.0.0 · oauthjs/node-oauth2-server#L158-L163

---

What is the reason for not unified with Promise.reject() ?
(Not limited to AuthorizeHandler.prototype.handle)

Thanks in advance.

[acandylevey]

Agreed, also it seems that error handling in general could be better.
Inside model.getClient
return Promise.reject(false) - Works correctly and throws invalid_credentials error.
return Promise.reject(new Error('Reason')) - Instead throws an invalid_grants error.

It would be really nice when Errors are returned, the handler should reject and pass the error back up the chain.