feat: removes redundant error message prefixes (#213)

passageidentity · Dec 30, 2024 · 72b2292 · 72b2292
1 parent c9fede5
commit 72b2292
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 32 deletions.
diff --git a/src/classes/Auth/Auth.ts b/src/classes/Auth/Auth.ts
@@ -46,7 +46,7 @@ export class Auth extends PassageBase {
 
         const { kid } = decodeProtectedHeader(jwt);
         if (!kid) {
-            throw new Error('Could not find valid cookie for authentication. You must catch this error.');
+            throw new Error('kid missing in jwt header.');
         }
 
         const {
@@ -94,7 +94,7 @@ export class Auth extends PassageBase {
 
             return response.magicLink;
         } catch (err) {
-            throw await this.parseError(err, 'Could not create a magic link for this app');
+            throw await this.parseError(err);
         }
     }
 }
diff --git a/src/classes/PassageBase/PassageBase.ts b/src/classes/PassageBase/PassageBase.ts
@@ -14,12 +14,11 @@ export class PassageBase {
     /**
      * Handle errors from PassageFlex API
      * @param {unknown} err error from node-fetch request
-     * @param {string} message optional message to include in the error
      * @return {Promise<void>}
      */
-    protected async parseError(err: unknown, message?: string): Promise<Error> {
+    protected async parseError(err: unknown): Promise<Error> {
         if (err instanceof ResponseError) {
-            throw await PassageError.fromResponseError(err, message);
+            throw await PassageError.fromResponseError(err);
         }
         return err as Error;
     }

diff --git a/src/classes/PassageError/PassageError.spec.ts b/src/classes/PassageError/PassageError.spec.ts
@@ -3,19 +3,18 @@ import { ResponseError } from '../../generated';
 
 describe('PassageError', () => {
     it('should map a ResponseError to a PassageError using fromResponseError', async () => {
-        const message = 'Test error message';
         const responseError: ResponseError = {
             response: {
                 status: 500,
                 json: async () => ({ code: 'INTERNAL_SERVER_ERROR', error: 'Internal server error' }),
             },
             message: 'Internal server error',
         } as ResponseError;
-        const passageError = await PassageError.fromResponseError(responseError, message);
+        const passageError = await PassageError.fromResponseError(responseError);
 
         expect(passageError).toBeInstanceOf(PassageError);
         expect(passageError.name).toBe('PassageError');
-        expect(passageError.message).toBe(`${message}: Internal server error`);
+        expect(passageError.message).toBe('Internal server error');
         expect(passageError.errorCode).toBe('INTERNAL_SERVER_ERROR');
         expect(passageError.statusCode).toBe(500);
     });

diff --git a/src/classes/PassageError/PassageError.ts b/src/classes/PassageError/PassageError.ts
@@ -29,9 +29,8 @@ export class PassageError extends Error {
      * @param {string} message Optional message to prefix the error message
      * @return {PassageError} PassageError
      */
-    public static async fromResponseError(err: ResponseError, message?: string): Promise<PassageError> {
+    public static async fromResponseError(err: ResponseError): Promise<PassageError> {
         const response: { code: string; error: string } = await err.response.json();
-        const formattedMessage = [message, response.error].filter(Boolean).join(': ');
-        return new PassageError(formattedMessage, response.code, err);
+        return new PassageError(response.error, response.code, err);
     }
 }
diff --git a/src/classes/User/User.spec.ts b/src/classes/User/User.spec.ts
@@ -2,6 +2,7 @@ import { User } from './User';
 import { PassageInstanceConfig } from '../PassageBase';
 import { UsersApi, ResponseError, ListPaginatedUsersResponse, Configuration } from '../../generated';
 import { PassageUser } from './types';
+import { PassageError } from '../PassageError';
 
 jest.mock('../../generated/apis');
 
@@ -39,24 +40,33 @@ describe('User class', () => {
     });
 
     it('should throw an error if get user by identifier fails', async () => {
-        usersApiMock.listPaginatedUsers.mockRejectedValue(
+        const responseError = new ResponseError(
+            {
+                status: 502,
+                json: async () => ({ code: 'error_code', error: 'Bad gateway' }),
+            } as Response,
+            'Error',
+        );
+        const passageError = await PassageError.fromResponseError(responseError);
+        usersApiMock.listPaginatedUsers.mockRejectedValue(responseError);
+
+        await expect(user.getByIdentifier('[email protected]')).rejects.toThrow(passageError);
+        await expect(user.getByIdentifier('[email protected]')).rejects.toThrow('Bad gateway');
+    });
+
+    it('should throw an error if get user by identifier returns an empty array', async () => {
+        const passageError = await PassageError.fromResponseError(
             new ResponseError(
                 {
                     status: 404,
-                    json: async () => ({ code: 'NOT_FOUND', error: 'Resource not found' }),
+                    json: async () => ({ code: 'user_not_found', error: 'User not found.' }),
                 } as Response,
                 'Error',
             ),
         );
-        await expect(user.getByIdentifier('[email protected]')).rejects.toThrow(Error);
-        await expect(user.getByIdentifier('[email protected]')).rejects.toThrow(
-            'Could not fetch user by identifier: Resource not found',
-        );
-    });
 
-    it('should throw an error if get user by identifier returns an empty array', async () => {
         usersApiMock.listPaginatedUsers.mockResolvedValue({ users: [] } as unknown as ListPaginatedUsersResponse);
-        await expect(user.getByIdentifier('[email protected]')).rejects.toThrow(Error);
+        await expect(user.getByIdentifier('[email protected]')).rejects.toThrow(passageError);
         await expect(user.getByIdentifier('[email protected]')).rejects.toThrow('User not found.');
     });
 });
diff --git a/src/classes/User/User.ts b/src/classes/User/User.ts
@@ -1,5 +1,5 @@
 import { PassageBase, PassageInstanceConfig } from '../PassageBase';
-import { TokensApi, UserDevicesApi, UsersApi, WebAuthnDevices } from '../../generated';
+import { ResponseError, TokensApi, UserDevicesApi, UsersApi, WebAuthnDevices } from '../../generated';
 import { CreateUserArgs, PassageUser, UpdateUserArgs } from './types';
 
 /**
@@ -40,7 +40,7 @@ export class User extends PassageBase {
 
             return response.user;
         } catch (err) {
-            throw await this.parseError(err, 'Could not fetch user');
+            throw await this.parseError(err);
         }
     }
 
@@ -64,12 +64,14 @@ export class User extends PassageBase {
 
             const users = response.users;
             if (!users.length) {
-                throw new Error('User not found.');
+                throw new ResponseError(
+                    new Response('{"code":"user_not_found","error":"User not found."}', { status: 404 }),
+                );
             }
 
             return this.get(users[0].id);
         } catch (err) {
-            throw await this.parseError(err, 'Could not fetch user by identifier');
+            throw await this.parseError(err);
         }
     }
 
@@ -91,7 +93,7 @@ export class User extends PassageBase {
             });
             return response.user;
         } catch (err) {
-            throw await this.parseError(err, 'Could not activate user');
+            throw await this.parseError(err);
         }
     }
 
@@ -114,7 +116,7 @@ export class User extends PassageBase {
 
             return response.user;
         } catch (err) {
-            throw await this.parseError(err, 'Could not deactivate user');
+            throw await this.parseError(err);
         }
     }
 
@@ -139,7 +141,7 @@ export class User extends PassageBase {
 
             return response.user;
         } catch (err) {
-            throw await this.parseError(err, 'Could not update user');
+            throw await this.parseError(err);
         }
     }
 
@@ -162,7 +164,7 @@ export class User extends PassageBase {
 
             return response.user;
         } catch (err) {
-            throw await this.parseError(err, 'Could not create user');
+            throw await this.parseError(err);
         }
     }
 
@@ -184,7 +186,7 @@ export class User extends PassageBase {
             });
             return true;
         } catch (err) {
-            throw await this.parseError(err, 'Could not delete user');
+            throw await this.parseError(err);
         }
     }
 
@@ -207,7 +209,7 @@ export class User extends PassageBase {
 
             return response.devices;
         } catch (err) {
-            throw await this.parseError(err, "Could not fetch user's devices:");
+            throw await this.parseError(err);
         }
     }
 
@@ -236,7 +238,7 @@ export class User extends PassageBase {
 
             return true;
         } catch (err) {
-            throw await this.parseError(err, "Could not delete user's device:");
+            throw await this.parseError(err);
         }
     }
 
@@ -258,7 +260,7 @@ export class User extends PassageBase {
             });
             return true;
         } catch (err) {
-            throw await this.parseError(err, "Could not revoke user's refresh tokens:");
+            throw await this.parseError(err);
         }
     }
 }