Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions src/common/filters/http-global-exception.filter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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<string, unknown> = {};
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',
Expand Down
30 changes: 28 additions & 2 deletions src/common/filters/http-global-exception.filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,45 @@ 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<string, unknown> | 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<string, unknown>;
return {
name: typeof e['name'] === 'string' ? e['name'] : 'UnknownError',
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'
? {
Expand Down Expand Up @@ -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',
);
Expand Down Expand Up @@ -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,
);
Expand Down
5 changes: 5 additions & 0 deletions src/modules/email/email.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
48 changes: 48 additions & 0 deletions src/modules/email/email.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ import {
NotFoundException,
BadRequestException,
ConflictException,
HttpException,
HttpStatus,
UnprocessableEntityException,
} from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { createMock, type DeepMocked } from '@golevelup/ts-vitest';
import { Readable } from 'node:stream';
import { EmailService } from './email.service.js';
import {
AttachmentUploadLimitError,
DraftUpdateConflictError,
MailProvider,
SendEmailFailedError,
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 12 additions & 2 deletions src/modules/email/email.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
BadRequestException,
ConflictException,
HttpException,
HttpStatus,
Injectable,
Logger,
NotFoundException,
Expand All @@ -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,
Expand Down Expand Up @@ -469,10 +472,17 @@ export class EmailService {
return this.mail.markAsFlagged(userEmail, id, flagged);
}

uploadAttachment(
async uploadAttachment(
payload: UploadAttachmentPayload,
): Promise<UploadAttachmentResponse> {
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(
Expand Down
14 changes: 14 additions & 0 deletions src/modules/email/mail-provider.port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down
Loading
Loading