From aed6ddbfb4b9a51576250b16d7b21d28b096bce8 Mon Sep 17 00:00:00 2001 From: Roy Scheeren Date: Thu, 1 Feb 2024 15:59:45 +0100 Subject: [PATCH 1/2] feat(envited.ascs.digital): structured logger Signed-off-by: Roy Scheeren --- .github/workflows/deploy-staging.yml | 1 + .../common/constants/errors.ts | 13 ++ .../common/logger/index.ts | 2 + .../common/logger/logger.ts | 11 ++ .../common/serverActions/profiles/get.test.ts | 35 +++- .../common/serverActions/profiles/get.ts | 17 +- .../serverActions/profiles/update.test.ts | 46 +++-- .../common/serverActions/profiles/update.ts | 36 ++-- .../users/deleteUserById.test.ts | 27 ++- .../serverActions/users/deleteUserById.ts | 23 ++- .../serverActions/users/getUserById.test.ts | 28 ++- .../common/serverActions/users/getUserById.ts | 22 ++- .../users/getUsersByIssuerId.test.ts | 26 ++- .../serverActions/users/getUsersByIssuerId.ts | 17 +- .../common/serverActions/users/insert.ts | 18 +- .../common/utils/errors.ts | 161 +++++++++++++++++- .../common/utils/index.ts | 4 +- .../modules/Users/Users.actions.ts | 9 +- 18 files changed, 396 insertions(+), 100 deletions(-) create mode 100644 apps/envited.ascs.digital/common/logger/index.ts create mode 100644 apps/envited.ascs.digital/common/logger/logger.ts diff --git a/.github/workflows/deploy-staging.yml b/.github/workflows/deploy-staging.yml index aeecb396..4c63384f 100644 --- a/.github/workflows/deploy-staging.yml +++ b/.github/workflows/deploy-staging.yml @@ -3,6 +3,7 @@ on: push: branches: - develop + - 66_stuctured_logger # Concurrency group name ensures concurrent workflow runs wait for any in-progress job to finish concurrency: diff --git a/apps/envited.ascs.digital/common/constants/errors.ts b/apps/envited.ascs.digital/common/constants/errors.ts index b064b74c..afc2ec26 100644 --- a/apps/envited.ascs.digital/common/constants/errors.ts +++ b/apps/envited.ascs.digital/common/constants/errors.ts @@ -1,3 +1,16 @@ export const ERRORS = { CANNOT_CONNECT_TO_DATABASE: 'Cannot connect to database', + UNAUTHORIZED: 'Unauthorized', + BAD_REQUEST: 'Bad Request', + FORBIDDEN: 'Forbidden', + INTERNAL_SERVER_ERROR: 'Internal Server Error', + NOT_FOUND: 'Not found', +} + +export const ERROR_CODES = { + BAD_REQUEST: 400, + UNAUTHORIZED: 401, + FORBIDDEN: 403, + NOT_FOUND: 404, + INTERNAL_SERVER_ERROR: 500, } diff --git a/apps/envited.ascs.digital/common/logger/index.ts b/apps/envited.ascs.digital/common/logger/index.ts new file mode 100644 index 00000000..00c6c2fa --- /dev/null +++ b/apps/envited.ascs.digital/common/logger/index.ts @@ -0,0 +1,2 @@ +export { log } from './logger' +export type { Log } from './logger' diff --git a/apps/envited.ascs.digital/common/logger/logger.ts b/apps/envited.ascs.digital/common/logger/logger.ts new file mode 100644 index 00000000..9af22e27 --- /dev/null +++ b/apps/envited.ascs.digital/common/logger/logger.ts @@ -0,0 +1,11 @@ +export interface Log { + info: typeof console.info + warn: typeof console.warn + error: typeof console.error +} + +export const log = { + info: console.info, + warn: console.warn, + error: console.error, +} diff --git a/apps/envited.ascs.digital/common/serverActions/profiles/get.test.ts b/apps/envited.ascs.digital/common/serverActions/profiles/get.test.ts index aa9a869f..acb30312 100644 --- a/apps/envited.ascs.digital/common/serverActions/profiles/get.test.ts +++ b/apps/envited.ascs.digital/common/serverActions/profiles/get.test.ts @@ -1,3 +1,4 @@ +import { ERRORS } from '../../constants' import { Role } from '../../types' import * as SUT from './get' @@ -32,10 +33,13 @@ describe('serverActions/profiles/get', () => { }, ]), }) + const logStub = { + error: jest.fn(), + } as any const slug = 'PROFILE_SLUG' - const result = await SUT._get({ db: dbStub, getServerSession: getServerSessionStub })(slug) + const result = await SUT._get({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(slug) const db = await dbStub() expect(result).toEqual({ name: 'USER_PRINCIPAL_NAME', @@ -77,10 +81,13 @@ describe('serverActions/profiles/get', () => { }, ]), }) + const logStub = { + error: jest.fn(), + } as any const slug = 'PROFILE_SLUG' - const result = await SUT._get({ db: dbStub, getServerSession: getServerSessionStub })(slug) + const result = await SUT._get({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(slug) const db = await dbStub() expect(result).toEqual({ name: 'USER_PRINCIPAL_NAME', @@ -115,10 +122,13 @@ describe('serverActions/profiles/get', () => { }, ]), }) + const logStub = { + error: jest.fn(), + } as any const slug = 'PROFILE_SLUG' - const result = await SUT._get({ db: dbStub, getServerSession: getServerSessionStub })(slug) + const result = await SUT._get({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(slug) const db = await dbStub() expect(result).toEqual({ name: 'USER_NAME', @@ -140,15 +150,20 @@ describe('serverActions/profiles/get', () => { }, }, ]) + const logStub = { + error: jest.fn(), + } as any const dbStub = jest.fn().mockResolvedValue({}) const slug = '' - expect.assertions(3) - await expect(() => SUT._get({ db: dbStub, getServerSession: getServerSessionStub })(slug)).rejects.toThrow() + await expect( + SUT._get({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(slug), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) expect(getServerSessionStub).not.toHaveBeenCalledWith() expect(dbStub).not.toHaveBeenCalled() + expect(logStub.error).toHaveBeenCalledWith({ message: 'Missing slug', name: 'BadRequestError' }) }) }) @@ -161,16 +176,22 @@ describe('serverActions/profiles/get', () => { role: Role.principal, }, }) + const logStub = { + error: jest.fn(), + } as any const dbStub = jest.fn().mockResolvedValue({ - getProfileBySlug: jest.fn().mockResolvedValue(null), + getProfileBySlug: jest.fn().mockResolvedValue([]), }) const slug = 'NON_EXISTANT_PROFILE_SLUG' const db = await dbStub() - await expect(() => SUT._get({ db: dbStub, getServerSession: getServerSessionStub })(slug)).rejects.toThrow() + await expect(SUT._get({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(slug)).rejects.toThrow( + ERRORS.INTERNAL_SERVER_ERROR, + ) expect(getServerSessionStub).toHaveBeenCalledWith() expect(dbStub).toHaveBeenCalledWith() expect(db.getProfileBySlug).toHaveBeenCalledWith(slug) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Not found', name: 'NotFoundError' }) }) }) diff --git a/apps/envited.ascs.digital/common/serverActions/profiles/get.ts b/apps/envited.ascs.digital/common/serverActions/profiles/get.ts index f3d1fadd..88d151a7 100644 --- a/apps/envited.ascs.digital/common/serverActions/profiles/get.ts +++ b/apps/envited.ascs.digital/common/serverActions/profiles/get.ts @@ -7,15 +7,16 @@ import { RESTRICTED_PROFILE_FIELDS } from '../../constants' import { db } from '../../database/queries' import { Database } from '../../database/types' import { isOwnProfile, isUsersCompanyProfile } from '../../guards' +import { Log, log } from '../../logger' import { Session } from '../../types' -import { badRequestError, error, notFoundError } from '../../utils' +import { badRequestError, formatError, internalServerErrorError, notFoundError } from '../../utils' export const _get = - ({ db, getServerSession }: { db: Database; getServerSession: () => Promise }) => + ({ db, getServerSession, log }: { db: Database; getServerSession: () => Promise; log: Log }) => async (slug: string) => { try { if (isNil(slug) || isEmpty(slug)) { - throw badRequestError('Missing slug') + throw badRequestError({ resource: 'profiles', resourceId: slug, message: 'Missing slug' }) } const session = await getServerSession() @@ -23,7 +24,7 @@ export const _get = const [profile] = await connection.getProfileBySlug(slug) if (isNil(profile) || isEmpty(profile)) { - throw notFoundError() + throw notFoundError({ resource: 'profiles', resourceId: slug, userId: session?.user.id }) } if (!isNil(session)) { @@ -40,10 +41,10 @@ export const _get = } return omit(RESTRICTED_PROFILE_FIELDS)(profile) - } catch (e) { - console.log('error', e) - throw error() + } catch (error: unknown) { + log.error(formatError(error)) + throw internalServerErrorError() } } -export const get = _get({ db, getServerSession }) +export const get = _get({ db, getServerSession, log }) diff --git a/apps/envited.ascs.digital/common/serverActions/profiles/update.test.ts b/apps/envited.ascs.digital/common/serverActions/profiles/update.test.ts index 3aa4e40b..ed87458c 100644 --- a/apps/envited.ascs.digital/common/serverActions/profiles/update.test.ts +++ b/apps/envited.ascs.digital/common/serverActions/profiles/update.test.ts @@ -1,3 +1,4 @@ +import { ERRORS } from '../../constants' import { Role } from '../../types' import * as SUT from './update' @@ -29,8 +30,11 @@ describe('serverActions/profiles/update', () => { updateProfile: jest.fn().mockResolvedValue([newProfile]), maybeUpdatePublishedState: jest.fn().mockResolvedValue([newProfile]), }) + const logStub = { + error: jest.fn(), + } as any - const result = await SUT._update({ db: dbStub, getServerSession: getServerSessionStub })(newProfile) + const result = await SUT._update({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(newProfile) const db = await dbStub() expect(result).toEqual(newProfile) expect(db.getUserWithProfileById).toHaveBeenCalledWith('USER_PKH') @@ -58,10 +62,14 @@ describe('serverActions/profiles/update', () => { getUserWithProfileById: jest.fn().mockResolvedValue([user]), updateProfile: jest.fn().mockResolvedValue(newProfile), }) + const logStub = { + error: jest.fn(), + } as any - await expect(() => - SUT._update({ db: dbStub, getServerSession: getServerSessionStub })(newProfile), - ).rejects.toThrow() + await expect( + SUT._update({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(newProfile), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Unauthorized', name: 'UnauthorizedError' }) }) it('should throw with incorrect role', async () => { @@ -88,10 +96,14 @@ describe('serverActions/profiles/update', () => { getUserWithProfileById: jest.fn().mockResolvedValue([user]), updateProfile: jest.fn().mockResolvedValue(newProfile), }) + const logStub = { + error: jest.fn(), + } as any - await expect(() => - SUT._update({ db: dbStub, getServerSession: getServerSessionStub })(newProfile), - ).rejects.toThrow() + await expect( + SUT._update({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(newProfile), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Incorrect role', name: 'ForbiddenError' }) }) it('should throw when user is not found', async () => { @@ -113,10 +125,14 @@ describe('serverActions/profiles/update', () => { getUserWithProfileById: jest.fn().mockResolvedValue([user]), updateProfile: jest.fn().mockResolvedValue(newProfile), }) + const logStub = { + error: jest.fn(), + } as any - await expect(() => - SUT._update({ db: dbStub, getServerSession: getServerSessionStub })(newProfile), - ).rejects.toThrow() + await expect( + SUT._update({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(newProfile), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'User not found', name: 'BadRequestError' }) }) it('should throw if user is updating someone elses profile', async () => { @@ -144,9 +160,13 @@ describe('serverActions/profiles/update', () => { getUserWithProfileById: jest.fn().mockResolvedValue([user]), updateProfile: jest.fn().mockResolvedValue(newProfile), }) + const logStub = { + error: jest.fn(), + } as any - await expect(() => - SUT._update({ db: dbStub, getServerSession: getServerSessionStub })(newProfile), - ).rejects.toThrow() + await expect( + SUT._update({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(newProfile), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Incorrect profile', name: 'ForbiddenError' }) }) }) diff --git a/apps/envited.ascs.digital/common/serverActions/profiles/update.ts b/apps/envited.ascs.digital/common/serverActions/profiles/update.ts index 72cb9fd4..4d0f8a5d 100644 --- a/apps/envited.ascs.digital/common/serverActions/profiles/update.ts +++ b/apps/envited.ascs.digital/common/serverActions/profiles/update.ts @@ -6,40 +6,56 @@ import { getServerSession } from '../../auth' import { db } from '../../database/queries' import { Database } from '../../database/types' import { isFederator, isOwnProfile, isPrincipal } from '../../guards' +import { Log, log } from '../../logger' import { Profile, Session } from '../../types' -import { badRequestError, error, unauthorizedError } from '../../utils' +import { badRequestError, forbiddenError, formatError, internalServerErrorError, unauthorizedError } from '../../utils' export const _update = - ({ db, getServerSession }: { db: Database; getServerSession: () => Promise }) => + ({ db, getServerSession, log }: { db: Database; getServerSession: () => Promise; log: Log }) => async (profile: Partial) => { try { const session = await getServerSession() if (isNil(session)) { - throw unauthorizedError() + throw unauthorizedError({ resource: 'profiles', resourceId: profile.id }) } if (!isFederator(session) && !isPrincipal(session)) { - throw badRequestError('Incorrect role') + throw forbiddenError({ + resource: 'profiles', + resourceId: profile.id, + message: 'Incorrect role', + userId: session.user.id, + }) } const connection = await db() const [user] = await connection.getUserWithProfileById(session.user.id) if (isNil(user)) { - throw badRequestError('User not found') + throw badRequestError({ + resource: 'profiles', + resourceId: profile.id, + message: 'User not found', + userId: session.user.id, + }) } if (!isOwnProfile(user)(profile)) { - throw badRequestError('Incorrect profile') + throw forbiddenError({ + resource: 'profiles', + resourceId: profile.id, + message: 'Incorrect profile', + userId: session.user.id, + }) } const [updatedProfile] = await connection.updateProfile(profile) const [result] = await connection.maybeUpdatePublishedState(updatedProfile) return result - } catch (e) { - console.log('error', e) - throw error() + } catch (error: unknown) { + log.error(formatError(error)) + throw internalServerErrorError() } } -export const update = _update({ db, getServerSession }) +export const update = _update({ db, getServerSession, log }) diff --git a/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.test.ts b/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.test.ts index cfc25ffa..f105ba91 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.test.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.test.ts @@ -1,3 +1,4 @@ +import { ERRORS } from '../../constants' import * as SUT from './deleteUserById' describe('common/serverActions/users/deleteUserById', () => { @@ -21,8 +22,13 @@ describe('common/serverActions/users/deleteUserById', () => { getUserById: jest.fn().mockResolvedValue([user]), deleteUserById: jest.fn().mockResolvedValue([{ updatedId: 'USER_PKH' }]), }) + const logStub = { + error: jest.fn(), + } as any - const result = await SUT._deleteUserById({ db: dbStub, getServerSession: getServerSessionStub })('USER_ID') + const result = await SUT._deleteUserById({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })( + 'USER_ID', + ) expect(result).toEqual(expected) }) @@ -38,10 +44,14 @@ describe('common/serverActions/users/deleteUserById', () => { getUserById: jest.fn().mockResolvedValue([user]), deleteUserById: jest.fn().mockResolvedValue([{ updatedId: 'USER_PKH' }]), }) + const logStub = { + error: jest.fn(), + } as any await expect( - SUT._deleteUserById({ db: dbStub, getServerSession: getServerSessionStub })('USER_ID'), - ).rejects.toThrow('Something went wrong') + SUT._deleteUserById({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })('USER_ID'), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Unauthorized', name: 'UnauthorizedError' }) }) it('should throw because requester is not allowed to get this resource', async () => { @@ -60,9 +70,16 @@ describe('common/serverActions/users/deleteUserById', () => { getUserById: jest.fn().mockResolvedValue([user]), deleteUserById: jest.fn().mockResolvedValue([{ updatedId: 'USER_PKH' }]), }) + const logStub = { + error: jest.fn(), + } as any await expect( - SUT._deleteUserById({ db: dbStub, getServerSession: getServerSessionStub })('USER_ID'), - ).rejects.toThrow('Something went wrong') + SUT._deleteUserById({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })('USER_ID'), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ + message: 'Not allowed to delete this resource', + name: 'ForbiddenError', + }) }) }) diff --git a/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.ts b/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.ts index 042b6a0b..d54a11e5 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/deleteUserById.ts @@ -1,38 +1,43 @@ import { isNil } from 'ramda' -import { cache } from 'react' import { getServerSession } from '../../auth' import { db } from '../../database/queries' import { Database } from '../../database/types' import { userIsIssuedByLoggedInUser } from '../../guards' +import { Log, log } from '../../logger' import { User } from '../../types' import { Session } from '../../types/types' -import { badRequestError, error, unauthorizedError } from '../../utils' +import { forbiddenError, formatError, internalServerErrorError, unauthorizedError } from '../../utils' export const _deleteUserById = - ({ db, getServerSession }: { db: Database; getServerSession: () => Promise }) => + ({ db, getServerSession, log }: { db: Database; getServerSession: () => Promise; log: Log }) => async (id: string): Promise => { try { const session = await getServerSession() if (isNil(session)) { - throw unauthorizedError() + throw unauthorizedError({ resource: 'users', resourceId: id }) } const connection = await db() const [user] = await connection.getUserById(id) if (!userIsIssuedByLoggedInUser(user)(session)) { - throw badRequestError('Incorrect permissions') + throw forbiddenError({ + resource: 'users', + resourceId: id, + message: 'Not allowed to delete this resource', + userId: session.user.id, + }) } const [deletedUser] = await connection.deleteUserById(user.id) return deletedUser - } catch (e) { - console.log('error', e) - throw error() + } catch (error: unknown) { + log.error(formatError(error)) + throw internalServerErrorError() } } -export const deleteUserById = cache(_deleteUserById({ db, getServerSession })) +export const deleteUserById = _deleteUserById({ db, getServerSession, log }) diff --git a/apps/envited.ascs.digital/common/serverActions/users/getUserById.test.ts b/apps/envited.ascs.digital/common/serverActions/users/getUserById.test.ts index e07e778a..8e246eb9 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/getUserById.test.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/getUserById.test.ts @@ -1,3 +1,4 @@ +import { ERRORS } from '../../constants' import * as SUT from './getUserById' describe('common/serverActions/users/getUserById', () => { @@ -23,8 +24,13 @@ describe('common/serverActions/users/getUserById', () => { const dbStub = jest.fn().mockResolvedValue({ getUserById: jest.fn().mockResolvedValue([user]), }) + const logStub = { + error: jest.fn(), + } as any - const result = await SUT._getUserById({ db: dbStub, getServerSession: getServerSessionStub })('USER_ID') + const result = await SUT._getUserById({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })( + 'USER_ID', + ) expect(result).toEqual(user) }) @@ -46,10 +52,14 @@ describe('common/serverActions/users/getUserById', () => { const dbStub = jest.fn().mockResolvedValue({ getUserById: jest.fn().mockResolvedValue([user]), }) + const logStub = { + error: jest.fn(), + } as any - await expect(SUT._getUserById({ db: dbStub, getServerSession: getServerSessionStub })('USER_ID')).rejects.toThrow( - 'Something went wrong', - ) + await expect( + SUT._getUserById({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })('USER_ID'), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Unauthorized', name: 'UnauthorizedError' }) }) it('should throw because requester is not allowed to get this resource', async () => { @@ -75,9 +85,13 @@ describe('common/serverActions/users/getUserById', () => { const dbStub = jest.fn().mockResolvedValue({ getUserById: jest.fn().mockResolvedValue([user]), }) + const logStub = { + error: jest.fn(), + } as any - await expect(SUT._getUserById({ db: dbStub, getServerSession: getServerSessionStub })('USER_ID')).rejects.toThrow( - 'Something went wrong', - ) + await expect( + SUT._getUserById({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })('USER_ID'), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Not allowed to get this resource', name: 'ForbiddenError' }) }) }) diff --git a/apps/envited.ascs.digital/common/serverActions/users/getUserById.ts b/apps/envited.ascs.digital/common/serverActions/users/getUserById.ts index e0939cf8..4591fa9c 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/getUserById.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/getUserById.ts @@ -5,32 +5,38 @@ import { getServerSession } from '../../auth' import { db } from '../../database/queries' import { Database } from '../../database/types' import { isOwnUser, userIsIssuedByLoggedInUser } from '../../guards' +import { Log, log } from '../../logger' import { User } from '../../types' import { Session } from '../../types/types' -import { badRequestError, error, unauthorizedError } from '../../utils' +import { forbiddenError, formatError, internalServerErrorError, unauthorizedError } from '../../utils' export const _getUserById = - ({ db, getServerSession }: { db: Database; getServerSession: () => Promise }) => + ({ db, getServerSession, log }: { db: Database; getServerSession: () => Promise; log: Log }) => async (id: string): Promise => { try { const session = await getServerSession() if (isNil(session)) { - throw unauthorizedError() + throw unauthorizedError({ resource: 'users', resourceId: id }) } const connection = await db() const [user] = await connection.getUserById(id) if (!userIsIssuedByLoggedInUser(user)(session) && !isOwnUser(user)(session)) { - throw badRequestError('Incorrect permissions') + throw forbiddenError({ + resource: 'users', + resourceId: id, + message: 'Not allowed to get this resource', + userId: session.user.id, + }) } return user - } catch (e) { - console.log('error', e) - throw error() + } catch (error: unknown) { + log.error(formatError(error)) + throw internalServerErrorError() } } -export const getUserById = cache(_getUserById({ db, getServerSession })) +export const getUserById = cache(_getUserById({ db, getServerSession, log })) diff --git a/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.test.ts b/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.test.ts index a1e2c047..64ec4a6e 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.test.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.test.ts @@ -1,3 +1,4 @@ +import { ERRORS } from '../../constants' import { Role } from '../../types' import * as SUT from './getUsersByIssuerId' @@ -29,8 +30,11 @@ describe('common/serverAction/users/getUsersByIssuerId', () => { const dbStub = jest.fn().mockResolvedValue({ getUsersByIssuerId: jest.fn().mockResolvedValue(user), }) + const logStub = { + error: jest.fn(), + } as any - const result = await SUT._getUsersByIssuerId({ db: dbStub, getServerSession: getServerSessionStub })() + const result = await SUT._getUsersByIssuerId({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })() expect(result).toEqual(user) }) @@ -55,10 +59,14 @@ describe('common/serverAction/users/getUsersByIssuerId', () => { const dbStub = jest.fn().mockResolvedValue({ getUsersByIssuerId: jest.fn().mockResolvedValue(user), }) + const logStub = { + error: jest.fn(), + } as any - await expect(SUT._getUsersByIssuerId({ db: dbStub, getServerSession: getServerSessionStub })()).rejects.toThrow( - 'Something went wrong', - ) + await expect( + SUT._getUsersByIssuerId({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Unauthorized', name: 'UnauthorizedError' }) }) it('should throw because requester is not allowed to get this resource', async () => { @@ -86,9 +94,13 @@ describe('common/serverAction/users/getUsersByIssuerId', () => { const dbStub = jest.fn().mockResolvedValue({ getUsersByIssuerId: jest.fn().mockResolvedValue(user), }) + const logStub = { + error: jest.fn(), + } as any - await expect(SUT._getUsersByIssuerId({ db: dbStub, getServerSession: getServerSessionStub })()).rejects.toThrow( - 'Something went wrong', - ) + await expect( + SUT._getUsersByIssuerId({ db: dbStub, getServerSession: getServerSessionStub, log: logStub })(), + ).rejects.toThrow(ERRORS.INTERNAL_SERVER_ERROR) + expect(logStub.error).toHaveBeenCalledWith({ message: 'Incorrect role', name: 'ForbiddenError' }) }) }) diff --git a/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.ts b/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.ts index 1328adee..d9849e2d 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/getUsersByIssuerId.ts @@ -5,32 +5,33 @@ import { getServerSession } from '../../auth' import { db } from '../../database/queries' import { Database } from '../../database/types' import { isFederator, isPrincipal } from '../../guards' +import { Log, log } from '../../logger' import { User } from '../../types' import { Session } from '../../types/types' -import { badRequestError, error, unauthorizedError } from '../../utils' +import { forbiddenError, formatError, internalServerErrorError, unauthorizedError } from '../../utils' export const _getUsersByIssuerId = - ({ db, getServerSession }: { db: Database; getServerSession: () => Promise }) => + ({ db, getServerSession, log }: { db: Database; getServerSession: () => Promise; log: Log }) => async (): Promise => { try { const session = await getServerSession() if (isNil(session)) { - throw unauthorizedError() + throw unauthorizedError({ resource: 'users' }) } if (!isFederator(session) && !isPrincipal(session)) { - throw badRequestError('Bad request') + throw forbiddenError({ resource: 'users', message: 'Incorrect role', userId: session.user.id }) } const connection = await db() const users = await connection.getUsersByIssuerId(session?.user?.pkh) return users - } catch (e) { - console.log('error', e) - throw error() + } catch (error: unknown) { + log.error(formatError(error)) + throw internalServerErrorError() } } -export const getUsersByIssuerId = cache(_getUsersByIssuerId({ db, getServerSession })) +export const getUsersByIssuerId = cache(_getUsersByIssuerId({ db, getServerSession, log })) diff --git a/apps/envited.ascs.digital/common/serverActions/users/insert.ts b/apps/envited.ascs.digital/common/serverActions/users/insert.ts index 23fb840c..e88aaf3c 100644 --- a/apps/envited.ascs.digital/common/serverActions/users/insert.ts +++ b/apps/envited.ascs.digital/common/serverActions/users/insert.ts @@ -4,17 +4,19 @@ import { equals, isEmpty } from 'ramda' import { db } from '../../database/queries' import { Credential, Database } from '../../database/types' +import { Log, log } from '../../logger' import { CredentialType } from '../../types' import { badRequestError, - error, extractIdFromCredential, extractIssuerIdFromCredential, extractTypeFromCredential, + formatError, + internalServerError, } from '../../utils' export const _insert = - ({ db }: { db: Database }) => + ({ db, log }: { db: Database; log: Log }) => async (credential: Credential) => { try { const connection = await db() @@ -26,21 +28,21 @@ export const _insert = const user = await connection.getUserById(credentialId) if (!isEmpty(user)) { - throw badRequestError('User already exists') + throw badRequestError({ resource: 'users', resourceId: credentialId, message: 'User already exists' }) } if (equals(CredentialType.AscsUser)(credentialType as CredentialType)) { const principal = await connection.getUserById(credentialIssuerId) if (isEmpty(principal)) { - throw badRequestError('Issuer not found') + throw badRequestError({ resource: 'users', resourceId: credentialIssuerId, message: 'Principal not found' }) } } return connection.insertUserTx(credential) - } catch (e) { - console.log('error', e) - throw error() + } catch (error: unknown) { + log.error(formatError(error)) + throw internalServerError() } } -export const insert = _insert({ db }) +export const insert = _insert({ db, log }) diff --git a/apps/envited.ascs.digital/common/utils/errors.ts b/apps/envited.ascs.digital/common/utils/errors.ts index dd447d5b..5020df9d 100644 --- a/apps/envited.ascs.digital/common/utils/errors.ts +++ b/apps/envited.ascs.digital/common/utils/errors.ts @@ -1,9 +1,160 @@ -export const unauthorized = () => new Error('Unauthorized') +import { assoc, propOr } from 'ramda' -export const badRequest = (message: string) => new Error(`BAD_REQUEST::403::${message}`) +import { ERRORS } from '../constants' +import { ERROR_CODES } from '../constants/errors' -export const internalServerError = () => new Error('Internal Server Error') +export class ExtendedError extends Error { + code: number | undefined + resource: string | undefined + resourceId: string | number | undefined + userId: string | undefined +} -export const notFound = () => new Error('Not Found') +class UnauthorizedError extends ExtendedError { + constructor({ + resource, + resourceId, + message, + userId, + }: { + resource: string + resourceId?: string | number + message: string + userId?: string + }) { + super(message) + this.name = 'UnauthorizedError' + this.code = ERROR_CODES.UNAUTHORIZED + this.resource = resource + this.resourceId = resourceId + this.userId = userId + } +} +class BadRequestError extends ExtendedError { + constructor({ + resource, + resourceId, + message, + userId, + }: { + resource: string + resourceId?: string | number + message: string + userId?: string + }) { + super(message) + this.name = 'BadRequestError' + this.code = ERROR_CODES.BAD_REQUEST + this.resource = resource + this.resourceId = resourceId + this.userId = userId + } +} -export const error = () => new Error('Something went wrong') +class ForbiddenError extends ExtendedError { + constructor({ + resource, + resourceId, + message, + userId, + }: { + resource: string + resourceId?: string | number + message: string + userId?: string + }) { + super(message) + this.name = 'ForbiddenError' + this.code = ERROR_CODES.BAD_REQUEST + this.resource = resource + this.resourceId = resourceId + this.userId = userId + } +} + +class NotFoundError extends ExtendedError { + constructor({ + resource, + resourceId, + message, + userId, + }: { + resource: string + resourceId?: string | number + message: string + userId?: string + }) { + super(message) + this.name = 'NotFoundError' + this.resource = resource + this.resourceId = resourceId + this.code = ERROR_CODES.BAD_REQUEST + this.userId = userId + } +} + +class InternalServerError extends ExtendedError { + constructor(message: string) { + super(message) + this.name = 'InternalServerError' + this.code = ERROR_CODES.INTERNAL_SERVER_ERROR + } +} + +export const formatError = (error: unknown) => { + const errorMessage = { + message: propOr('Unknown error message', 'message')(error), + name: propOr('Unknown error name', 'name')(error), + } + if (error instanceof ExtendedError) { + assoc('code', error.code)(errorMessage) + } + + return errorMessage +} + +export const unauthorized = ({ + resource, + resourceId, + userId, +}: { + resource: string + resourceId?: string | number + userId?: string +}) => new UnauthorizedError({ resource, resourceId, message: ERRORS.UNAUTHORIZED, userId }) + +export const badRequest = ({ + resource, + resourceId, + message, + userId, +}: { + resource: string + resourceId?: string | number + message: string + userId?: string +}) => new BadRequestError({ resource, resourceId, message, userId }) + +export const forbidden = ({ + resource, + resourceId, + message, + userId, +}: { + resource: string + resourceId?: string | number + message: string + userId?: string +}) => new ForbiddenError({ resource, resourceId, message, userId }) + +export const internalServerError = () => new InternalServerError(ERRORS.INTERNAL_SERVER_ERROR) + +export const notFound = ({ + resource, + resourceId, + userId, +}: { + resource: string + resourceId?: string | number + userId?: string +}) => new NotFoundError({ resource, resourceId, message: ERRORS.NOT_FOUND, userId }) diff --git a/apps/envited.ascs.digital/common/utils/index.ts b/apps/envited.ascs.digital/common/utils/index.ts index 9733fb64..9222595c 100644 --- a/apps/envited.ascs.digital/common/utils/index.ts +++ b/apps/envited.ascs.digital/common/utils/index.ts @@ -5,7 +5,9 @@ export { internalServerError as internalServerErrorError, notFound as notFoundError, unauthorized as unauthorizedError, - error, + ExtendedError, + formatError, + forbidden as forbiddenError, } from './errors' export { extractIdFromCredential, extractIssuerIdFromCredential, extractTypeFromCredential, slugify } from './utils' diff --git a/apps/envited.ascs.digital/modules/Users/Users.actions.ts b/apps/envited.ascs.digital/modules/Users/Users.actions.ts index b6e04a08..d905a1b1 100644 --- a/apps/envited.ascs.digital/modules/Users/Users.actions.ts +++ b/apps/envited.ascs.digital/modules/Users/Users.actions.ts @@ -2,17 +2,18 @@ import { revalidatePath } from 'next/cache' +import { log } from '../../common/logger' import { deleteUserById } from '../../common/serverActions/users/deleteUserById' -import { error } from '../../common/utils' +import { formatError, internalServerErrorError } from '../../common/utils' export async function deleteUser(id: string) { try { await deleteUserById(id) revalidatePath('/dashboard/users') - } catch (e) { - console.log('error', e) + } catch (error: unknown) { + log.error(formatError(error)) - throw error() + throw internalServerErrorError() } } From 334c1d790c69f15b0679629dd22219df6c0cad44 Mon Sep 17 00:00:00 2001 From: Roy Scheeren Date: Thu, 1 Feb 2024 16:15:36 +0100 Subject: [PATCH 2/2] refactor(envited.ascs.digital): remove branch deployment Signed-off-by: Roy Scheeren --- .github/workflows/deploy-staging.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/deploy-staging.yml b/.github/workflows/deploy-staging.yml index 4c63384f..aeecb396 100644 --- a/.github/workflows/deploy-staging.yml +++ b/.github/workflows/deploy-staging.yml @@ -3,7 +3,6 @@ on: push: branches: - develop - - 66_stuctured_logger # Concurrency group name ensures concurrent workflow runs wait for any in-progress job to finish concurrency: