From e1d479ab6450d26f26fe1e54192925fdb5d0aad0 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Wed, 19 Mar 2025 19:45:03 +0700 Subject: [PATCH] Uberf-9663: Improve mail logging (#8275) * UBERF-9663: Improve mail logging Signed-off-by: Artem Savchenko * UBERF-9663: Fix log message Signed-off-by: Artem Savchenko * UBERF-9663: Use context for logging Signed-off-by: Artem Savchenko * UBERF-9663: Add context Signed-off-by: Artem Savchenko * UBERF-9663: Fix test Signed-off-by: Artem Savchenko * UBERF-9663: Use measure context for logging Signed-off-by: Artem Savchenko * UBERF-9663: Configure logger Signed-off-by: Artem Savchenko * UBERF-9663: Additional logger info Signed-off-by: Artem Savchenko --------- Signed-off-by: Artem Savchenko --- server-plugins/gmail-resources/src/index.ts | 5 ++- server/account/src/__tests__/utils.test.ts | 10 ++++- server/account/src/operations.ts | 17 +++++--- server/account/src/utils.ts | 17 ++++++-- services/mail/pod-mail/package.json | 3 ++ .../mail/pod-mail/src/__tests__/main.test.ts | 29 ++++++++----- services/mail/pod-mail/src/mail.ts | 8 ++-- services/mail/pod-mail/src/main.ts | 42 ++++++++++++++----- 8 files changed, 95 insertions(+), 36 deletions(-) diff --git a/server-plugins/gmail-resources/src/index.ts b/server-plugins/gmail-resources/src/index.ts index c6e380cdee..d3b7eeddf9 100644 --- a/server-plugins/gmail-resources/src/index.ts +++ b/server-plugins/gmail-resources/src/index.ts @@ -118,7 +118,7 @@ export async function sendEmailNotification ( return } const mailAuth: string | undefined = getMetadata(serverNotification.metadata.MailAuthToken) - await fetch(concatLink(mailURL, '/send'), { + const response = await fetch(concatLink(mailURL, '/send'), { method: 'post', keepalive: true, headers: { @@ -132,6 +132,9 @@ export async function sendEmailNotification ( to: [receiver] }) }) + if (!response.ok) { + ctx.error(`Failed to send email notification: ${response.statusText}`) + } } catch (err) { ctx.error('Could not send email notification', { err, receiver }) } diff --git a/server/account/src/__tests__/utils.test.ts b/server/account/src/__tests__/utils.test.ts index 928a2ae607..288dd31856 100644 --- a/server/account/src/__tests__/utils.test.ts +++ b/server/account/src/__tests__/utils.test.ts @@ -753,7 +753,9 @@ describe('account utils', () => { describe('sendOtpEmail', () => { beforeEach(() => { - global.fetch = jest.fn() + const mockFetch = jest.fn() + mockFetch.mockResolvedValue({ ok: true }) + global.fetch = mockFetch }) test('should send email with OTP', async () => { @@ -1010,6 +1012,10 @@ describe('account utils', () => { }) test('should send email with correct parameters', async () => { + const mockCtx = { + error: jest.fn(), + info: jest.fn() + } as unknown as MeasureContext const emailInfo = { text: 'Test email', html: '

Test email

', @@ -1017,7 +1023,7 @@ describe('account utils', () => { to: 'test@example.com' } - await sendEmail(emailInfo) + await sendEmail(emailInfo, mockCtx) expect(mockFetch).toHaveBeenCalledWith('https://ses.example.com/send', { method: 'post', diff --git a/server/account/src/operations.ts b/server/account/src/operations.ts index ae5fa6c27a..4782cc87f3 100644 --- a/server/account/src/operations.ts +++ b/server/account/src/operations.ts @@ -466,7 +466,7 @@ export async function sendInvite ( const inviteId = await createInviteLink(ctx, db, branding, token, { exp, emailMask: email, limit: 1, role }) const inviteEmail = await getInviteEmail(branding, email, inviteId, workspace, expHours) - await sendEmail(inviteEmail) + await sendEmail(inviteEmail, ctx) ctx.info('Invite has been sent', { to: inviteEmail.to, workspaceUuid: workspace.uuid, workspaceName: workspace.name }) } @@ -533,7 +533,7 @@ export async function resendInvite ( } const inviteEmail = await getInviteEmail(branding, email, inviteId, workspace, expHours, true) - await sendEmail(inviteEmail) + await sendEmail(inviteEmail, ctx) ctx.info('Invite has been resent', { to: inviteEmail.to, @@ -773,7 +773,7 @@ export async function requestPasswordReset ( const html = await translate(accountPlugin.string.RecoveryHTML, { link }, lang) const subject = await translate(accountPlugin.string.RecoverySubject, {}, lang) - await fetch(concatLink(mailURL, '/send'), { + const response = await fetch(concatLink(mailURL, '/send'), { method: 'post', headers: { 'Content-Type': 'application/json', @@ -786,8 +786,15 @@ export async function requestPasswordReset ( to: normalizedEmail }) }) - - ctx.info('Password reset email sent', { email, normalizedEmail, account: account.uuid }) + if (response.ok) { + ctx.info('Password reset email sent', { email, normalizedEmail, account: account.uuid }) + } else { + ctx.error(`Failed to send reset password email: ${response.statusText}`, { + email, + normalizedEmail, + account: account.uuid + }) + } } export async function restorePassword ( diff --git a/server/account/src/utils.ts b/server/account/src/utils.ts index 435b27f7ca..4b57bd76df 100644 --- a/server/account/src/utils.ts +++ b/server/account/src/utils.ts @@ -405,7 +405,7 @@ export async function sendOtpEmail ( const subject = await translate(accountPlugin.string.OtpSubject, { code: otp, app }, lang) const to = email - await fetch(concatLink(mailURL, '/send'), { + const response = await fetch(concatLink(mailURL, '/send'), { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -418,6 +418,9 @@ export async function sendOtpEmail ( to }) }) + if (!response.ok) { + ctx.error(`Failed to send otp email: ${response.statusText}`, { to }) + } } export async function isOtpValid (db: AccountDB, socialId: string, code: string): Promise { @@ -782,7 +785,7 @@ export async function sendEmailConfirmation ( const html = await translate(accountPlugin.string.ConfirmationHTML, { name, link }, lang) const subject = await translate(accountPlugin.string.ConfirmationSubject, { name }, lang) - await fetch(concatLink(mailURL, '/send'), { + const response = await fetch(concatLink(mailURL, '/send'), { method: 'post', headers: { 'Content-Type': 'application/json', @@ -795,6 +798,9 @@ export async function sendEmailConfirmation ( to: email }) }) + if (!response.ok) { + ctx.error(`Failed to send email confirmation: ${response.statusText}`, { email }) + } } export async function confirmEmail (ctx: MeasureContext, db: AccountDB, account: string, email: string): Promise { @@ -1165,10 +1171,10 @@ interface EmailInfo { to: string } -export async function sendEmail (info: EmailInfo): Promise { +export async function sendEmail (info: EmailInfo, ctx: MeasureContext): Promise { const { text, html, subject, to } = info const { mailURL, mailAuth } = getMailUrl() - await fetch(concatLink(mailURL, '/send'), { + const response = await fetch(concatLink(mailURL, '/send'), { method: 'post', headers: { 'Content-Type': 'application/json', @@ -1181,6 +1187,9 @@ export async function sendEmail (info: EmailInfo): Promise { to }) }) + if (!response.ok) { + ctx.error(`Failed to send mail: ${response.statusText}`, { to }) + } } export function sanitizeEmail (email: string): string { diff --git a/services/mail/pod-mail/package.json b/services/mail/pod-mail/package.json index adfa35700a..fe3a7e9822 100644 --- a/services/mail/pod-mail/package.json +++ b/services/mail/pod-mail/package.json @@ -55,6 +55,9 @@ "dependencies": { "@aws-sdk/client-ses": "^3.738.0", "@types/nodemailer": "^6.4.17", + "@hcengineering/analytics-service": "^0.6.0", + "@hcengineering/core": "^0.6.32", + "@hcengineering/server-core": "^0.6.1", "cors": "^2.8.5", "dotenv": "~16.0.0", "express": "^4.21.2", diff --git a/services/mail/pod-mail/src/__tests__/main.test.ts b/services/mail/pod-mail/src/__tests__/main.test.ts index ea740f3b80..a5524f73ae 100644 --- a/services/mail/pod-mail/src/__tests__/main.test.ts +++ b/services/mail/pod-mail/src/__tests__/main.test.ts @@ -14,6 +14,7 @@ // import { Request, Response } from 'express' +import { type MeasureContext } from '@hcengineering/core' import { MailClient } from '../mail' import { handleSendMail } from '../main' @@ -31,6 +32,7 @@ describe('handleSendMail', () => { let res: Response let sendMailMock: jest.Mock let mailClient: MailClient + let mockCtx: MeasureContext beforeEach(() => { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions @@ -49,12 +51,16 @@ describe('handleSendMail', () => { mailClient = new MailClient() sendMailMock = (mailClient.sendMessage as jest.Mock).mockResolvedValue({}) + mockCtx = { + info: jest.fn(), + error: jest.fn() + } as unknown as MeasureContext }) it('should return 400 if text is missing', async () => { req.body.text = undefined - await handleSendMail(new MailClient(), req, res) + await handleSendMail(new MailClient(), req, res, mockCtx) // eslint-disable-next-line @typescript-eslint/unbound-method expect(res.status).toHaveBeenCalledWith(400) @@ -64,7 +70,7 @@ describe('handleSendMail', () => { it('should return 400 if subject is missing', async () => { req.body.subject = undefined - await handleSendMail(new MailClient(), req, res) + await handleSendMail(new MailClient(), req, res, mockCtx) // eslint-disable-next-line @typescript-eslint/unbound-method expect(res.status).toHaveBeenCalledWith(400) @@ -74,7 +80,7 @@ describe('handleSendMail', () => { it('should return 400 if to is missing', async () => { req.body.to = undefined - await handleSendMail(new MailClient(), req, res) + await handleSendMail(new MailClient(), req, res, mockCtx) // eslint-disable-next-line @typescript-eslint/unbound-method expect(res.status).toHaveBeenCalledWith(400) @@ -84,13 +90,13 @@ describe('handleSendMail', () => { it('handles errors thrown by MailClient', async () => { sendMailMock.mockRejectedValue(new Error('Email service error')) - await handleSendMail(new MailClient(), req, res) + await handleSendMail(new MailClient(), req, res, mockCtx) expect(res.send).toHaveBeenCalled() // Check that a response is still sent }) it('should use source from config if from is not provided', async () => { - await handleSendMail(mailClient, req, res) + await handleSendMail(mailClient, req, res, mockCtx) expect(sendMailMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -98,13 +104,14 @@ describe('handleSendMail', () => { to: 'test@example.com', subject: 'Test Subject', text: 'Hello, world!' - }) + }), + mockCtx ) }) it('should use from if it is provided', async () => { req.body.from = 'test.from@example.com' - await handleSendMail(mailClient, req, res) + await handleSendMail(mailClient, req, res, mockCtx) expect(sendMailMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -112,13 +119,14 @@ describe('handleSendMail', () => { to: 'test@example.com', subject: 'Test Subject', text: 'Hello, world!' - }) + }), + mockCtx ) }) it('should send to multiple addresses', async () => { req.body.to = ['test1@example.com', 'test2@example.com'] - await handleSendMail(mailClient, req, res) + await handleSendMail(mailClient, req, res, mockCtx) expect(sendMailMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -126,7 +134,8 @@ describe('handleSendMail', () => { to: ['test1@example.com', 'test2@example.com'], // Verify that multiple addresses are passed subject: 'Test Subject', text: 'Hello, world!' - }) + }), + mockCtx ) }) }) diff --git a/services/mail/pod-mail/src/mail.ts b/services/mail/pod-mail/src/mail.ts index ca7d312020..258827ee44 100644 --- a/services/mail/pod-mail/src/mail.ts +++ b/services/mail/pod-mail/src/mail.ts @@ -13,6 +13,7 @@ // limitations under the License. // import { type SendMailOptions, type Transporter } from 'nodemailer' +import { MeasureContext } from '@hcengineering/core' import config from './config' import { getTransport } from './transport' @@ -24,14 +25,13 @@ export class MailClient { this.transporter = getTransport(config) } - async sendMessage (message: SendMailOptions): Promise { + async sendMessage (message: SendMailOptions, ctx: MeasureContext): Promise { this.transporter.sendMail(message, (err, info) => { const messageInfo = `(from: ${message.from as string}, to: ${message.to as string})` if (err !== null) { - console.error(`Failed to send email ${messageInfo}: `, err.message) - console.log('Failed message details: ', message) + ctx.error(`Failed to send email ${messageInfo}: ${err.message}`) } else { - console.log(`Email request ${messageInfo} sent: ${info?.response}`) + ctx.info(`Email request ${messageInfo} sent: ${info?.response}`) } }) } diff --git a/services/mail/pod-mail/src/main.ts b/services/mail/pod-mail/src/main.ts index 423b457bdd..fa4cd55aca 100644 --- a/services/mail/pod-mail/src/main.ts +++ b/services/mail/pod-mail/src/main.ts @@ -16,6 +16,10 @@ import { type SendMailOptions } from 'nodemailer' import { Request, Response } from 'express' import Mail from 'nodemailer/lib/mailer' +import { initStatisticsContext } from '@hcengineering/server-core' +import { MeasureContext, MeasureMetricsContext, newMetrics } from '@hcengineering/core' +import { SplitLogger } from '@hcengineering/analytics-service' +import { join } from 'path' import config from './config' import { createServer, listen } from './server' @@ -23,15 +27,28 @@ import { MailClient } from './mail' import { Endpoint } from './types' export const main = async (): Promise => { + const measureCtx = initStatisticsContext('mail', { + factory: () => + new MeasureMetricsContext( + 'mail', + {}, + {}, + newMetrics(), + new SplitLogger('mail', { + root: join(process.cwd(), 'logs'), + enableConsole: (process.env.ENABLE_CONSOLE ?? 'true') === 'true' + }) + ) + }) const client = new MailClient() - console.log('Mail service has been started') + measureCtx.info('Mail service has been started') const endpoints: Endpoint[] = [ { endpoint: '/send', type: 'post', handler: async (req, res) => { - await handleSendMail(client, req, res) + await handleSendMail(client, req, res, measureCtx) } } ] @@ -46,15 +63,20 @@ export const main = async (): Promise => { process.on('SIGINT', shutdown) process.on('SIGTERM', shutdown) - process.on('uncaughtException', (e) => { - console.error(e) + process.on('uncaughtException', (e: any) => { + measureCtx.error(e.message) }) - process.on('unhandledRejection', (e) => { - console.error(e) + process.on('unhandledRejection', (e: any) => { + measureCtx.error(e.message) }) } -export async function handleSendMail (client: MailClient, req: Request, res: Response): Promise { +export async function handleSendMail ( + client: MailClient, + req: Request, + res: Response, + ctx: MeasureContext +): Promise { const { from, to, subject, text, html, attachments, headers, apiKey } = req.body if (process.env.API_KEY !== undefined && process.env.API_KEY !== apiKey) { res.status(401).send({ err: 'Unauthorized' }) @@ -93,9 +115,9 @@ export async function handleSendMail (client: MailClient, req: Request, res: Res message.attachments = getAttachments(attachments) } try { - await client.sendMessage(message) - } catch (err) { - console.log(err) + await client.sendMessage(message, ctx) + } catch (err: any) { + ctx.error(err.message) } res.send()