Skip to content

Commit

Permalink
feat(endorsement-system): Improved error logs (#16835)
Browse files Browse the repository at this point in the history 
* better error log

* chore: nx format:write update dirty files

* better error logs

* better error logs

* better error logs

* chore: nx format:write update dirty files

* better error logs

* chore: nx format:write update dirty files

* better error logs

* chore: nx format:write update dirty files

* dev bucket fix

* cleanup

* chore: nx format:write update dirty files

---------

Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
@rafnarnason @andes-it
@kodiakhq @jonnigs
3 people authored and jonnigs committed Nov 26, 2024
1 parent 542d8c7 commit 946c4ce
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 40 deletions.
120 changes: 81 additions & 39 deletions apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Injectable,
NotFoundException,
BadRequestException,
InternalServerErrorException,
} from '@nestjs/common'
import { InjectModel } from '@nestjs/sequelize'
import { Op } from 'sequelize'
Expand Down Expand Up @@ -192,11 +193,21 @@ export class EndorsementListService {
}

async lock(endorsementList: EndorsementList): Promise<EndorsementList> {
this.logger.info(`Locking endorsement list: ${endorsementList.id}`)
if (process.env.NODE_ENV === 'production') {
await this.emailLock(endorsementList)
try {
this.logger.info(`Locking endorsement list: ${endorsementList.id}`)
if (process.env.NODE_ENV === 'production') {
await this.emailLock(endorsementList)
}
return await endorsementList.update({ adminLock: true })
} catch (error) {
this.logger.error('Failed to lock endorsement list', {
error: error.message,
listId: endorsementList.id,
})
throw new InternalServerErrorException(
`Failed to lock endorsement list: ${error.message}`,
)
}
return await endorsementList.update({ adminLock: true })
}

async unlock(endorsementList: EndorsementList): Promise<EndorsementList> {
Expand Down Expand Up @@ -279,7 +290,7 @@ export class EndorsementListService {
return result
}

async getOwnerInfo(listId: string, owner?: string) {
async getOwnerInfo(listId: string, owner?: string): Promise<string> {
this.logger.info(`Finding single endorsement lists by id "${listId}"`)
if (!owner) {
const endorsementList = await this.endorsementListModel.findOne({
Expand All @@ -297,15 +308,12 @@ export class EndorsementListService {
try {
const person = await this.nationalRegistryApiV3.getName(owner)
return person?.fulltNafn ? person.fulltNafn : ''
} catch (e) {
if (e instanceof Error) {
this.logger.warn(
`Occured when fetching owner name from NationalRegistryApi ${e.message} \n${e.stack}`,
)
return ''
} else {
throw e
}
} catch (error) {
this.logger.error('Failed to fetch owner name from NationalRegistry', {
error: error.message,
listId,
})
return ''
}
}

Expand Down Expand Up @@ -592,7 +600,11 @@ export class EndorsementListService {
})
return { success: true }
} catch (error) {
this.logger.error('Failed to send email', error)
this.logger.error('Failed to send PDF email', {
error: error.message,
listId,
recipientEmail,
})
return { success: false }
}
}
Expand Down Expand Up @@ -668,7 +680,10 @@ export class EndorsementListService {
})
return { success: true }
} catch (error) {
this.logger.error('Failed to send email', error)
this.logger.error('Failed to send creation notification email', {
error: error.message,
listId: endorsementList.id,
})
return { success: false }
}
}
Expand Down Expand Up @@ -770,7 +785,10 @@ export class EndorsementListService {
})
return { success: true }
} catch (error) {
this.logger.error('Failed to send email', error)
this.logger.error('Failed to send creation notification email', {
error: error.message,
listId: endorsementList.id,
})
return { success: false }
}
}
Expand All @@ -780,44 +798,53 @@ export class EndorsementListService {
fileType: 'pdf' | 'csv',
): Promise<EndorsementListExportUrlResponse> {
try {
this.logger.info(`Exporting list ${listId} as ${fileType}`, { listId })

// Validate file type
if (!['pdf', 'csv'].includes(fileType)) {
throw new BadRequestException(
'Invalid file type. Allowed values are "pdf" or "csv".',
`Invalid file type. Allowed values are "pdf" or "csv"`,
)
}

// Fetch endorsement list
const endorsementList = await this.fetchEndorsementList(listId, user)
if (!endorsementList) {
throw new NotFoundException(
`Endorsement list ${listId} not found or access denied.`,
`Endorsement list ${listId} not found or access denied`,
)
}

// Create file buffer
const fileBuffer =
fileType === 'pdf'
? await this.createPdfBuffer(endorsementList)
: this.createCsvBuffer(endorsementList)

// Upload to S3
const filename = `undirskriftalisti-${listId}-${new Date()
.toISOString()
.replace(/[:.]/g, '-')}.${fileType}`

await this.uploadFileToS3(fileBuffer, filename, fileType)

// Generate presigned URL with 60 minutes expiration
const url = await this.s3Service.getPresignedUrl({
bucket: environment.exportsBucketName,
key: filename,
})

return { url }
} catch (error) {
this.logger.error(`Failed to export list ${listId}`, { error })
throw error
this.logger.error('Failed to export list', {
error: error.message,
listId,
fileType,
})

if (
error instanceof BadRequestException ||
error instanceof NotFoundException
) {
throw error // Re-throw validation errors
}

throw new InternalServerErrorException(
`Failed to export list: ${error.message}`,
)
}
}

Expand Down Expand Up @@ -858,14 +885,21 @@ export class EndorsementListService {
endorsementList,
ownerName,
)

if (!Buffer.isBuffer(pdfBuffer)) {
throw new InternalServerErrorException(
'Generated PDF is not a valid buffer',
)
}

return pdfBuffer
} catch (error) {
this.logger.error(
`Failed to create PDF buffer for endorsement list ${endorsementList.id}`,
{ error },
)
throw new Error(
`Error generating PDF for endorsement list ${endorsementList.id}`,
this.logger.error('Failed to create PDF buffer', {
error: error.message,
listId: endorsementList.id,
})
throw new InternalServerErrorException(
`Failed to generate PDF: ${error.message}`,
)
}
}
Expand All @@ -876,16 +910,24 @@ export class EndorsementListService {
fileType: 'pdf' | 'csv',
): Promise<void> {
try {
if (!environment.exportsBucketName) {
throw new InternalServerErrorException('S3 bucket name is undefined')
}

await this.s3Service.uploadFile(
fileBuffer,
{ bucket: environment.exportsBucketName, key: filename },
{
ContentType: fileType === 'pdf' ? 'application/pdf' : 'text/csv',
},
{ ContentType: fileType === 'pdf' ? 'application/pdf' : 'text/csv' },
)
} catch (error) {
this.logger.error(`Failed to upload file to S3`, { error, filename })
throw new Error('Error uploading file to S3')
this.logger.error('Failed to upload file to S3', {
error: error.message,
filename,
bucketName: environment.exportsBucketName,
})
throw new InternalServerErrorException(
`Failed to upload file to S3: ${error.message}`,
)
}
}
}
2 changes: 1 addition & 1 deletion apps/services/endorsements/api/src/environments/environment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const devConfig = {
sender: 'devland.is',
address: '[email protected]',
},
exportsBucketName: process.env.ENDORSEMENT_SYSTEM_EXPORTS_BUCKET_NAME,
exportsBucketName: 'island-is-dev-exports-endorsement-system',
}

const prodConfig = {
Expand Down

0 comments on commit 946c4ce

Please sign in to comment.