diff --git a/src/common/filters/http-global-exception.filter.spec.ts b/src/common/filters/http-global-exception.filter.spec.ts index 543a59b..21c8f7b 100644 --- a/src/common/filters/http-global-exception.filter.spec.ts +++ b/src/common/filters/http-global-exception.filter.spec.ts @@ -60,6 +60,34 @@ describe('HttpGlobalExceptionFilter', () => { ); }); + it('when an HttpException carries upstream details, then they are logged alongside the response', () => { + const mockException = Object.assign( + new HttpException('Attachment upload limit reached', 429), + { details: 'Quota exceeded: blob upload quota' }, + ); + const mockHost = createMockArgumentsHost('/email/attachment', 'POST'); + + filter.catch(mockException, mockHost); + + expect(loggerMock.error).toHaveBeenCalledWith( + expect.objectContaining({ + error: expect.objectContaining({ + details: 'Quota exceeded: blob upload quota', + }) as unknown, + }), + 'HTTP_EXCEPTION', + 'HttpGlobalExceptionFilter', + ); + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + { + statusCode: 429, + message: 'Attachment upload limit reached', + }, + 429, + ); + }); + describe('Non-HTTP errors', () => { it('when unexpected error is thrown, then logs details and returns 500', () => { const mockException = new Error('Unexpected error'); @@ -86,6 +114,67 @@ describe('HttpGlobalExceptionFilter', () => { ); }); + it('when an upstream error carries details, then the upstream body is logged', () => { + const upstreamBody = JSON.stringify({ + status: 403, + title: 'Quota exceeded', + }); + const mockException = Object.assign(new Error('Blob upload failed'), { + details: upstreamBody, + }); + const mockHost = createMockArgumentsHost('/email/attachment', 'POST'); + + filter.catch(mockException, mockHost); + + expect(loggerMock.error).toHaveBeenCalledWith( + expect.objectContaining({ + error: expect.objectContaining({ + details: upstreamBody, + }) as unknown, + }), + 'UNEXPECTED_ERROR', + 'HttpGlobalExceptionFilter', + ); + }); + + it('when the upstream details are an object, then they are serialized for the log', () => { + const mockException = Object.assign(new Error('JMAP method error'), { + details: [['error', { type: 'stateMismatch' }, 'r0']], + }); + const mockHost = createMockArgumentsHost('/email', 'POST'); + + filter.catch(mockException, mockHost); + + expect(loggerMock.error).toHaveBeenCalledWith( + expect.objectContaining({ + error: expect.objectContaining({ + details: '[["error",{"type":"stateMismatch"},"r0"]]', + }) as unknown, + }), + 'UNEXPECTED_ERROR', + 'HttpGlobalExceptionFilter', + ); + }); + + it('when the upstream details are unserializable, then logging still succeeds', () => { + const circular: Record = {}; + circular['self'] = circular; + const mockException = Object.assign(new Error('JMAP method error'), { + details: circular, + }); + const mockHost = createMockArgumentsHost('/email', 'POST'); + + filter.catch(mockException, mockHost); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + statusCode: HttpStatus.INTERNAL_SERVER_ERROR, + }), + HttpStatus.INTERNAL_SERVER_ERROR, + ); + }); + it('when SequelizeError is thrown, then logs with DATABASE tag', () => { const mockException = new ValidationError( 'Database validation error', diff --git a/src/common/filters/http-global-exception.filter.ts b/src/common/filters/http-global-exception.filter.ts index a864332..ee2265d 100644 --- a/src/common/filters/http-global-exception.filter.ts +++ b/src/common/filters/http-global-exception.filter.ts @@ -13,13 +13,37 @@ import type { UserPayload } from '../../modules/auth/jwt-payload.dto.js'; type AuthenticatedRequest = Request & { user?: UserPayload }; +const MAX_DETAILS_LENGTH = 2000; + interface ErrorLike { name: string; message: string; stack?: string; + details?: string; original?: { code?: string }; } +function toDetails(exception: unknown): string | undefined { + const details = (exception as Record | null)?.['details']; + + if (details == null) return undefined; + + const serialized = + typeof details === 'string' ? details : safeStringify(details); + + return serialized.length > MAX_DETAILS_LENGTH + ? `${serialized.slice(0, MAX_DETAILS_LENGTH)}…` + : serialized; +} + +function safeStringify(value: unknown): string { + try { + return JSON.stringify(value) ?? String(value); + } catch { + return String(value); + } +} + function toErrorLike(exception: unknown): ErrorLike { const e = exception as Record; return { @@ -27,6 +51,7 @@ function toErrorLike(exception: unknown): ErrorLike { message: typeof e['message'] === 'string' ? e['message'] : String(exception), stack: typeof e['stack'] === 'string' ? e['stack'] : undefined, + details: toDetails(exception), original: e['original'] != null && typeof e['original'] === 'object' ? { @@ -68,9 +93,10 @@ export class HttpGlobalExceptionFilter extends BaseExceptionFilter { this.logger.error( { requestId, + name: exception.name, path: request.url, method: request.method, - error: { message: res }, + error: { message: res, details: toDetails(exception) }, }, 'HTTP_EXCEPTION', ); @@ -215,7 +241,7 @@ export class HttpGlobalExceptionFilter extends BaseExceptionFilter { method: request.method, body: (request.body ?? {}) as unknown, user: { email: request.user?.email, uuid: request.user?.uuid }, - error: { message: err.message, stack: err.stack }, + error: { message: err.message, stack: err.stack, details: err.details }, }, errorCategory, ); diff --git a/src/modules/email/email.controller.ts b/src/modules/email/email.controller.ts index 7866815..c9ec408 100644 --- a/src/modules/email/email.controller.ts +++ b/src/modules/email/email.controller.ts @@ -29,6 +29,7 @@ import { ApiParam, ApiQuery, ApiTags, + ApiTooManyRequestsResponse, } from '@nestjs/swagger'; import { MailAddress } from '../account/decorators/mail-address.decorator.js'; import { MailAccountGuard } from '../provisioning/provisioning.guard.js'; @@ -347,6 +348,10 @@ export class EmailController { type: UploadAttachmentResponseDto, description: 'Upload attachment successfully', }) + @ApiTooManyRequestsResponse({ + description: + 'Upload allowance for the account is exhausted, retry in a few minutes', + }) async uploadAttachment( @UploadedFiles() files: Express.Multer.File[], @MailAddress('address') email: string, diff --git a/src/modules/email/email.service.spec.ts b/src/modules/email/email.service.spec.ts index 7763248..d83f9ef 100644 --- a/src/modules/email/email.service.spec.ts +++ b/src/modules/email/email.service.spec.ts @@ -5,6 +5,8 @@ import { NotFoundException, BadRequestException, ConflictException, + HttpException, + HttpStatus, UnprocessableEntityException, } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; @@ -12,6 +14,7 @@ import { createMock, type DeepMocked } from '@golevelup/ts-vitest'; import { Readable } from 'node:stream'; import { EmailService } from './email.service.js'; import { + AttachmentUploadLimitError, DraftUpdateConflictError, MailProvider, SendEmailFailedError, @@ -1049,6 +1052,51 @@ describe('EmailService', () => { }); }); + describe('uploadAttachment', () => { + const payload = { + userEmail, + blob: { + name: 'image.jpg', + buffer: Buffer.from('binary'), + mimeType: 'image/jpeg', + }, + }; + + it('when the attachment is stored, then the stored details are returned', async () => { + const stored = { + blobId: 'blob-1', + size: 6, + type: 'image/jpeg', + }; + provider.uploadAttachment.mockResolvedValue(stored); + + await expect(service.uploadAttachment(payload)).resolves.toBe(stored); + + expect(provider.uploadAttachment).toHaveBeenCalledWith(payload); + }); + + it('when the provider upload limit is reached, then the caller is asked to retry later', async () => { + provider.uploadAttachment.mockRejectedValue( + new AttachmentUploadLimitError(), + ); + + await expect(service.uploadAttachment(payload)).rejects.toMatchObject({ + status: HttpStatus.TOO_MANY_REQUESTS, + message: 'Attachment upload limit reached, please try again later', + }); + await expect(service.uploadAttachment(payload)).rejects.toBeInstanceOf( + HttpException, + ); + }); + + it('when the upload fails for any other reason, then the error is propagated', async () => { + const failure = new Error('upstream is down'); + provider.uploadAttachment.mockRejectedValue(failure); + + await expect(service.uploadAttachment(payload)).rejects.toBe(failure); + }); + }); + describe('markAsFlagged', () => { it('when called with false, then delegates to provider', async () => { provider.markAsFlagged.mockResolvedValue(undefined); diff --git a/src/modules/email/email.service.ts b/src/modules/email/email.service.ts index 3f9fbe4..05e78ba 100644 --- a/src/modules/email/email.service.ts +++ b/src/modules/email/email.service.ts @@ -1,6 +1,8 @@ import { BadRequestException, ConflictException, + HttpException, + HttpStatus, Injectable, Logger, NotFoundException, @@ -10,6 +12,7 @@ import { ConfigService } from '@nestjs/config'; import { AccountService } from '../account/account.service.js'; import { MailUsageService } from '../usage/mail-usage.service.js'; import { + AttachmentUploadLimitError, DraftUpdateConflictError, MailProvider, SendEmailFailedError, @@ -469,10 +472,17 @@ export class EmailService { return this.mail.markAsFlagged(userEmail, id, flagged); } - uploadAttachment( + async uploadAttachment( payload: UploadAttachmentPayload, ): Promise { - return this.mail.uploadAttachment(payload); + try { + return await this.mail.uploadAttachment(payload); + } catch (error) { + if (error instanceof AttachmentUploadLimitError) { + throw new HttpException(error.message, HttpStatus.TOO_MANY_REQUESTS); + } + throw error; + } } downloadAttachment( diff --git a/src/modules/email/mail-provider.port.ts b/src/modules/email/mail-provider.port.ts index 32bf62d..27f0e73 100644 --- a/src/modules/email/mail-provider.port.ts +++ b/src/modules/email/mail-provider.port.ts @@ -40,6 +40,20 @@ export class SendEmailFailedError extends Error { } } +/** + * The provider refused the upload because the account exhausted the upload + * allowance of the provider's current time window. The allowance frees itself + * when the window rolls over, so the caller may retry later. + */ +export class AttachmentUploadLimitError extends Error { + constructor() { + super('Attachment upload limit reached, please try again later'); + this.name = 'AttachmentUploadLimitError'; + + Object.setPrototypeOf(this, AttachmentUploadLimitError.prototype); + } +} + export class MissingMessageIdError extends UnprocessableEntityException { constructor(parentId: string) { super(`Original email ${parentId} has no Message-ID; cannot thread reply`); diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts index 8ad02b4..536fc14 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts @@ -9,6 +9,7 @@ import { JmapService, } from './jmap.service.js'; import { + AttachmentUploadLimitError, DraftUpdateConflictError, MissingMessageIdError, } from '../../email/mail-provider.port.js'; @@ -1101,15 +1102,17 @@ describe('JmapMailProvider', () => { }); describe('Uploading an attachment', () => { + const uploadPayload = { + userEmail: 'user@test.com', + blob: { + name: 'image.jpg', + buffer: Buffer.from('binary'), + mimeType: 'image/jpeg', + }, + }; + it('when a user uploads an attachment, then the file is forwarded for storage and the stored details are returned', async () => { - const payload = { - userEmail: 'user@test.com', - blob: { - name: 'image.jpg', - buffer: Buffer.from('binary'), - mimeType: 'image/jpeg', - }, - }; + const payload = uploadPayload; const storedBlob = { accountId: 'acc-1', blobId: 'blob-1', @@ -1123,6 +1126,62 @@ describe('JmapMailProvider', () => { expect(jmapService.uploadAttachment).toHaveBeenCalledWith(payload); expect(result).toBe(storedBlob); }); + + it('when the account ran out of upload allowance, then the upload fails with the port limit error', async () => { + jmapService.uploadAttachment.mockRejectedValue( + new JmapError( + 'Blob upload failed: HTTP 403', + JSON.stringify({ + type: 'about:blank', + status: 403, + title: 'Quota exceeded', + detail: + 'You have exceeded the blob upload quota of 1000 files or 50000000 bytes.', + }), + 403, + ), + ); + + await expect(provider.uploadAttachment(uploadPayload)).rejects.toThrow( + AttachmentUploadLimitError, + ); + }); + + it('when the upload quota error is not reported as JSON, then the raw upstream detail is still recognized', async () => { + jmapService.uploadAttachment.mockRejectedValue( + new JmapError('Blob upload failed: HTTP 403', 'Quota exceeded', 403), + ); + + await expect(provider.uploadAttachment(uploadPayload)).rejects.toThrow( + AttachmentUploadLimitError, + ); + }); + + it('when the upload is forbidden for a reason other than quota, then the original error is propagated', async () => { + const forbidden = new JmapError( + 'Blob upload failed: HTTP 403', + JSON.stringify({ status: 403, title: 'Forbidden' }), + 403, + ); + jmapService.uploadAttachment.mockRejectedValue(forbidden); + + await expect(provider.uploadAttachment(uploadPayload)).rejects.toBe( + forbidden, + ); + }); + + it('when the upload fails upstream, then the original error is propagated', async () => { + const failure = new JmapError( + 'Blob upload failed: HTTP 500', + 'server boom', + 500, + ); + jmapService.uploadAttachment.mockRejectedValue(failure); + + await expect(provider.uploadAttachment(uploadPayload)).rejects.toBe( + failure, + ); + }); }); describe('Downloading an attachment', () => { diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.ts index d119040..b9c895d 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.ts @@ -1,5 +1,6 @@ import { Injectable, Logger } from '@nestjs/common'; import { + AttachmentUploadLimitError, DraftUpdateConflictError, MailProvider, MissingMessageIdError, @@ -96,6 +97,30 @@ const isStateMismatchError = (err: unknown): boolean => (invocation[1] as { type?: string } | null)?.type === 'stateMismatch', ); +const HTTP_FORBIDDEN = 403; + +const readJmapErrorDetail = (details: unknown): string | null => { + if (typeof details !== 'string' || !details) return null; + + try { + const body = JSON.parse(details) as { title?: string; detail?: string }; + + return [body?.title, body?.detail].filter(Boolean).join(': ') || details; + } catch { + return details; + } +}; + +const uploadLimitDetail = (err: unknown): string | null => { + if (!(err instanceof JmapError) || err.statusCode !== HTTP_FORBIDDEN) { + return null; + } + + const detail = readJmapErrorDetail(err.details); + + return detail && /quota/i.test(detail) ? detail : null; +}; + @Injectable() export class JmapMailProvider extends MailProvider { private readonly logger = new Logger(JmapMailProvider.name); @@ -1013,14 +1038,22 @@ export class JmapMailProvider extends MailProvider { userEmail, blob, }: UploadAttachmentPayload): Promise { - return this.jmap.uploadAttachment({ - userEmail, - blob: { - name: blob.name, - buffer: blob.buffer, - mimeType: blob.mimeType, - }, - }); + try { + return await this.jmap.uploadAttachment({ + userEmail, + blob: { + name: blob.name, + buffer: blob.buffer, + mimeType: blob.mimeType, + }, + }); + } catch (error) { + const limitDetail = uploadLimitDetail(error); + + if (limitDetail) throw new AttachmentUploadLimitError(); + + throw error; + } } async downloadAttachment( diff --git a/src/modules/infrastructure/jmap/jmap.service.spec.ts b/src/modules/infrastructure/jmap/jmap.service.spec.ts index eb2c51f..a87d88b 100644 --- a/src/modules/infrastructure/jmap/jmap.service.spec.ts +++ b/src/modules/infrastructure/jmap/jmap.service.spec.ts @@ -151,6 +151,31 @@ describe('JMAP service', () => { ).rejects.toBeInstanceOf(JmapError); }); + it('when the attachment is rejected upstream, then the failure carries the upstream status and body', async () => { + const upstreamBody = { + type: 'about:blank', + status: 403, + title: 'Quota exceeded', + detail: + 'You have exceeded the blob upload quota of 1000 files or 50000000 bytes.', + }; + mockRequest.mockResolvedValueOnce(httpResponse(403, upstreamBody)); + + await expect( + service.uploadAttachment({ + userEmail, + blob: { + name: 'hello.pdf', + buffer: Buffer.from('x'), + mimeType: 'image/png', + }, + }), + ).rejects.toMatchObject({ + statusCode: 403, + details: JSON.stringify(upstreamBody), + }); + }); + it('when the user does not have a mail account, then the upload fails with an error', async () => { // override the session response queued in beforeEach mockRequest.mockReset(); diff --git a/src/modules/infrastructure/jmap/jmap.service.ts b/src/modules/infrastructure/jmap/jmap.service.ts index a3eb4c8..91fac43 100644 --- a/src/modules/infrastructure/jmap/jmap.service.ts +++ b/src/modules/infrastructure/jmap/jmap.service.ts @@ -205,7 +205,11 @@ export class JmapService implements OnModuleInit, OnModuleDestroy { const text = await body.text(); if (statusCode !== 200 && statusCode !== 201) { - throw new JmapError(`Blob upload failed: HTTP ${statusCode}`, text); + throw new JmapError( + `Blob upload failed: HTTP ${statusCode}`, + text, + statusCode, + ); } const data = JSON.parse(text) as { @@ -265,6 +269,7 @@ export class JmapError extends Error { constructor( message: string, public readonly details: unknown, + public readonly statusCode?: number, ) { super(message); this.name = 'JmapError';