Repository: badgateway/oauth2-client (ts)

Issue:
Handle non-oauth2 errors better
If a oauth2 server returns a HTTP error that's not in the standard oauth2 error format, this terrible error gets thrown:

```
  TypeError: Cannot read properties of undefined (reading 'error')
      at OAuth2Client.request (/home/evert/src/sproutfamily/app/node_modules/@badgateway/oauth2-client/dist/client.js:228:26)

```

Embarrassing!

Pull request:
Better error handling
More robust error handling. When an error is emitted, you now give you access to the emitted HTTP Response and response body.

Also, non-OAuth2 formatted errors were not handled nicely. This should make this a lot easier to handle.

Fixes #146 

Patch:
diff --git a/changelog.md b/changelog.md
index c497bff..8b6ca5e 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,6 +1,13 @@
 Changelog
 =========
 
+2.4.0 (????-??-??)
+------------------
+
+* More robust error handling. When an error is emitted, you now give you access
+  to the emitted HTTP Response and response body.
+
+
 2.3.0 (2024-02-03)
 ------------------
 
@@ -8,7 +15,7 @@ Changelog
   the `client_id`.
 * Support for the `resource` parameter from RFC 8707.
 * Add support for `scope` parameter to `refresh()`.
-* Support for RFC 7009, Token Revocation.
+* Support for RFC 7009, Token Revocation (@adambom).
 
 
 2.2.4 (2023-09-05)
diff --git a/src/client.ts b/src/client.ts
index ae00b2b..ba765df 100644
--- a/src/client.ts
+++ b/src/client.ts
@@ -11,7 +11,7 @@ import {
   ServerMetadataResponse,
   TokenResponse,
 } from './messages';
-import { OAuth2Error } from './error';
+import { OAuth2HttpError } from './error';
 import { OAuth2AuthorizationCodeClient } from './client/authorization-code';
 
 
@@ -414,7 +414,7 @@ export class OAuth2Client {
     });
 
     let responseBody;
-    if (resp.status !== 204 && resp.headers.has('Content-Type') && resp.headers.get('Content-Type')!.startsWith('application/json')) {
+    if (resp.status !== 204 && resp.headers.has('Content-Type') && resp.headers.get('Content-Type')!.match(/^application\/(.*\+)?json/)) {
       responseBody = await resp.json();
     }
     if (resp.ok) {
@@ -424,7 +424,7 @@ export class OAuth2Client {
     let errorMessage;
     let oauth2Code;
 
-    if (responseBody.error) {
+    if (responseBody?.error) {
       // This is likely an OAUth2-formatted error
       errorMessage = 'OAuth2 error ' + responseBody.error + '.';
       if (responseBody.error_description) {
@@ -439,7 +439,7 @@ export class OAuth2Client {
       }
       oauth2Code = null;
     }
-    throw new OAuth2Error(errorMessage, oauth2Code, resp.status);
+    throw new OAuth2HttpError(errorMessage, oauth2Code, resp, responseBody);
   }
 
   /**
diff --git a/src/client/authorization-code.ts b/src/client/authorization-code.ts
index 18d86c7..144db62 100644
--- a/src/client/authorization-code.ts
+++ b/src/client/authorization-code.ts
@@ -152,7 +152,6 @@ export class OAuth2AuthorizationCodeClient {
       throw new OAuth2Error(
         queryParams.get('error_description') ?? 'OAuth2 error',
         queryParams.get('error') as any,
-        0,
       );
     }
 
diff --git a/src/error.ts b/src/error.ts
index 69e8234..a825586 100644
--- a/src/error.ts
+++ b/src/error.ts
@@ -3,7 +3,7 @@ import { OAuth2ErrorCode } from './messages';
 /**
  * An error class for any error the server emits.
  *
- * The 'code' property will have the oauth2 error type,
+ * The 'oauth2Code' property will have the oauth2 error type,
  * such as:
  * - invalid_request
  * - invalid_client
@@ -14,15 +14,45 @@ import { OAuth2ErrorCode } from './messages';
  */
 export class OAuth2Error extends Error {
 
-  oauth2Code: OAuth2ErrorCode;
-  httpCode: number;
+  oauth2Code: OAuth2ErrorCode|string;
 
-  constructor(message: string, oauth2Code: OAuth2ErrorCode, httpCode: number) {
+  constructor(message: OAuth2ErrorCode|string, oauth2Code: OAuth2ErrorCode) {
 
     super(message);
-
     this.oauth2Code = oauth2Code;
-    this.httpCode = httpCode;
+
+  }
+
+}
+
+/**
+ * A OAuth2 error that was emitted as a HTTP error
+ *
+ * The 'code' property will have the oauth2 error type,
+ * such as:
+ * - invalid_request
+ * - invalid_client
+ * - invalid_grant
+ * - unauthorized_client
+ * - unsupported_grant_type
+ * - invalid_scope
+ *
+ * This Error also gives you access to the HTTP status code and response body.
+ */
+export class OAuth2HttpError extends OAuth2Error {
+
+  httpCode: number;
+
+  response: Response;
+  parsedBody: Record<string, any>;
+
+  constructor(message: string, oauth2Code: OAuth2ErrorCode, response: Response, parsedBody: Record<string, any>) {
+
+    super(message, oauth2Code);
+
+    this.httpCode = response.status;
+    this.response = response;
+    this.parsedBody = parsedBody;
 
   }
 
diff --git a/src/index.ts b/src/index.ts
index 7a59a4c..509bdd9 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -2,6 +2,6 @@ export { OAuth2Client } from './client';
 export { OAuth2AuthorizationCodeClient, generateCodeVerifier } from './client/authorization-code';
 export { OAuth2Fetch } from './fetch-wrapper';
 export { OAuth2Token } from './token';
-export { OAuth2Error } from './error';
+export { OAuth2Error, OAuth2HttpError } from './error';
 
 export { IntrospectionResponse } from './messages';


Test patch:
diff --git a/test/client-credentials.ts b/test/client-credentials.ts
index afba0e6..1c661cb 100644
--- a/test/client-credentials.ts
+++ b/test/client-credentials.ts
@@ -1,5 +1,5 @@
 import { testServer } from './test-server';
-import { OAuth2Client } from '../src';
+import { OAuth2Client, OAuth2HttpError } from '../src';
 import { expect } from 'chai';
 
 describe('client-credentials', () => {
@@ -121,5 +121,113 @@ describe('client-credentials', () => {
     });
   });
 
+  describe('error handling', async() => {
+
+    it('should create a OAuth2HttpError if an error was thrown', async() => {
+
+      const server = testServer();
+
+      const client = new OAuth2Client({
+        server: server.url,
+        tokenEndpoint: '/token',
+        clientId: 'oauth2-error',
+        clientSecret: 'test-client-secret',
+        authenticationMethod: 'client_secret_post'
+      });
+
+      const resource = [
+        'https://example/resource1',
+        'https://example/resource2',
+      ];
+
+      try {
+        await client.clientCredentials({
+          resource,
+        });
+        throw new Error('This operation should have failed');
+      } catch (err:any) {
+
+        expect(err).to.be.instanceof(OAuth2HttpError);
+        expect(err.response).to.be.instanceof(Response);
+        expect(err.oauth2Code).to.equal('invalid_client');
+        expect(err.parsedBody).to.deep.equal({
+          error: 'invalid_client',
+          error_description: 'OOps!',
+        });
+
+      }
+
+    });
+    it('should create a OAuth2HttpError also if a non-oauth2 error was thrown with a JSON response', async() => {
+
+      const server = testServer();
+
+      const client = new OAuth2Client({
+        server: server.url,
+        tokenEndpoint: '/token',
+        clientId: 'json-error',
+        clientSecret: 'test-client-secret',
+        authenticationMethod: 'client_secret_post'
+      });
+
+      const resource = [
+        'https://example/resource1',
+        'https://example/resource2',
+      ];
+
+      try {
+        await client.clientCredentials({
+          resource,
+        });
+        throw new Error('This operation should have failed');
+      } catch (err:any) {
+
+        expect(err).to.be.instanceof(OAuth2HttpError);
+        expect(err.response).to.be.instanceof(Response);
+        expect(err.httpCode).to.equal(418);
+        expect(err.oauth2Code).to.equal(null);
+        expect(err.parsedBody).to.deep.equal({
+          status: 418,
+          title: 'OOps!',
+          type: 'https://example/dummy',
+        });
+
+      }
+
+    });
+    it('should create a OAuth2HttpError when a generic HTTP error was thrown ', async() => {
+
+      const server = testServer();
+
+      const client = new OAuth2Client({
+        server: server.url,
+        tokenEndpoint: '/token',
+        clientId: 'general-http-error',
+        clientSecret: 'test-client-secret',
+        authenticationMethod: 'client_secret_post'
+      });
+
+      const resource = [
+        'https://example/resource1',
+        'https://example/resource2',
+      ];
+
+      try {
+        await client.clientCredentials({
+          resource,
+        });
+        throw new Error('This operation should have failed');
+      } catch (err:any) {
+
+        expect(err).to.be.instanceof(OAuth2HttpError);
+        expect(err.response).to.be.instanceof(Response);
+        expect(err.oauth2Code).to.equal(null);
+        expect(err.parsedBody).to.equal(undefined);
+
+      }
+
+    });
+
+  });
 
 });
diff --git a/test/test-server.ts b/test/test-server.ts
index 40cdefa..2ed52fd 100644
--- a/test/test-server.ts
+++ b/test/test-server.ts
@@ -26,6 +26,9 @@ export function testServer() {
     lastRequest = ctx.request;
     return next();
   });
+  app.use(oauth2Error);
+  app.use(jsonError);
+  app.use(generalHttpError);
   app.use(issueToken);
   app.use(revokeToken);
   app.use(discover);
@@ -51,7 +54,49 @@ export function testServer() {
 
 }
 
+const oauth2Error: Middleware = (ctx, next) => {
 
+  if (ctx.request.body.client_id !== 'oauth2-error') {
+    return next();
+  }
+
+  ctx.response.body = {
+    error: 'invalid_client',
+    error_description: 'OOps!',
+  };
+
+  ctx.response.status = 400;
+  ctx.response.type = 'application/json';
+
+
+};
+const jsonError: Middleware = (ctx, next) => {
+
+  if (ctx.request.body.client_id !== 'json-error') {
+    return next();
+  }
+
+  ctx.response.body = {
+    type: 'https://example/dummy',
+    title: 'OOps!',
+    status: 418,
+  };
+
+  ctx.response.status = 418;
+  ctx.response.type = 'application/problem+json';
+
+};
+const generalHttpError: Middleware = (ctx, next) => {
+
+  if (ctx.request.body.client_id !== 'general-http-error') {
+    return next();
+  }
+
+  ctx.response.body = 'We\'re super broken RN!';
+  ctx.response.status = 500;
+  ctx.response.type = 'text/plain';
+
+};
 
 const issueToken: Middleware = (ctx, next) => {