Skip to content

Commit d6cc777

Browse files
ArtyomSavchenkoArtyomSavchenko
authored
UBERF-9663: Improve mail logging (#8290)
Signed-off-by: Artem Savchenko <[email protected]>
1 parent 372c5d7 commit d6cc777

File tree

6 files changed

+103
-34
lines changed

6 files changed

+103
-34
lines changed

Diff for: ‎server-plugins/gmail-resources/src/index.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export async function sendEmailNotification (
117117
return
118118
}
119119
const mailAuth: string | undefined = getMetadata(serverNotification.metadata.MailAuthToken)
120-
await fetch(concatLink(mailURL, '/send'), {
120+
const response = await fetch(concatLink(mailURL, '/send'), {
121121
method: 'post',
122122
keepalive: true,
123123
headers: {
@@ -131,6 +131,9 @@ export async function sendEmailNotification (
131131
to: [receiver]
132132
})
133133
})
134+
if (!response.ok) {
135+
ctx.error(`Failed to send email notification: ${response.statusText}`)
136+
}
134137
} catch (err) {
135138
ctx.error('Could not send email notification', { err, receiver })
136139
}

Diff for: ‎server/account/src/operations.ts

+41-9
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ async function getAccountInfo (
179179
return toAccountInfo(account)
180180
}
181181

182-
async function sendOtpEmail (branding: Branding | null, otp: string, email: string): Promise<void> {
182+
async function sendOtpEmail (ctx: MeasureContext, branding: Branding | null, otp: string, email: string): Promise<void> {
183183
const mailURL = getMetadata(accountPlugin.metadata.MAIL_URL)
184184
if (mailURL === undefined || mailURL === '') {
185185
console.info('Please provide email service url to enable email otp.')
@@ -194,7 +194,7 @@ async function sendOtpEmail (branding: Branding | null, otp: string, email: stri
194194
const subject = await translate(accountPlugin.string.OtpSubject, { code: otp, app }, lang)
195195

196196
const to = email
197-
await fetch(concatLink(mailURL, '/send'), {
197+
const response = await fetch(concatLink(mailURL, '/send'), {
198198
method: 'POST',
199199
headers: {
200200
'Content-Type': 'application/json'
@@ -206,6 +206,12 @@ async function sendOtpEmail (branding: Branding | null, otp: string, email: stri
206206
to
207207
})
208208
})
209+
if (!response.ok) {
210+
ctx.error('Failed to send OTP email', {
211+
email,
212+
error: response.statusText
213+
})
214+
}
209215
}
210216

211217
export async function getAccountInfoByToken (
@@ -318,7 +324,7 @@ export async function sendOtp (
318324
const expires = now + timeToLive
319325
const otp = await getNewOtp(db)
320326

321-
await sendOtpEmail(branding, otp, email)
327+
await sendOtpEmail(ctx, branding, otp, email)
322328
await db.otp.insertOne({ account: account._id, otp, expires, createdOn: now })
323329

324330
return { sent: true, retryOn: now + retryDelay * 1000 }
@@ -2299,7 +2305,7 @@ export async function requestPassword (
22992305
const subject = await translate(accountPlugin.string.RecoverySubject, {}, lang)
23002306

23012307
const to = account.email
2302-
await fetch(concatLink(mailURL, '/send'), {
2308+
const response = await fetch(concatLink(mailURL, '/send'), {
23032309
method: 'post',
23042310
headers: {
23052311
'Content-Type': 'application/json'
@@ -2311,7 +2317,15 @@ export async function requestPassword (
23112317
to
23122318
})
23132319
})
2314-
ctx.info('recovery email sent', { email, accountEmail: account.email })
2320+
if (response.ok) {
2321+
ctx.info('recovery email sent', { email, accountEmail: account.email })
2322+
} else {
2323+
ctx.error('Failed to send reset password email', {
2324+
email,
2325+
accountEmail: account.email,
2326+
error: response.statusText
2327+
})
2328+
}
23152329
}
23162330

23172331
/**
@@ -2561,7 +2575,7 @@ export async function sendInvite (
25612575
const subject = await translate(accountPlugin.string.InviteSubject, { ws }, lang)
25622576

25632577
const to = email
2564-
await fetch(concatLink(mailURL, '/send'), {
2578+
const response = await fetch(concatLink(mailURL, '/send'), {
25652579
method: 'post',
25662580
headers: {
25672581
'Content-Type': 'application/json'
@@ -2573,7 +2587,16 @@ export async function sendInvite (
25732587
to
25742588
})
25752589
})
2576-
ctx.info('Invite sent', { email, workspace, link })
2590+
if (response.ok) {
2591+
ctx.info('Invite sent', { email, workspace, link })
2592+
} else {
2593+
ctx.error('Failed to send invite email', {
2594+
email,
2595+
workspace,
2596+
link,
2597+
error: response.statusText
2598+
})
2599+
}
25772600
}
25782601

25792602
async function checkSendRateLimit (currentAccount: Account, workspace: string, db: AccountDB): Promise<void> {
@@ -2645,7 +2668,7 @@ export async function resendInvite (
26452668
const subject = await translate(accountPlugin.string.ResendInviteSubject, { ws }, lang)
26462669

26472670
const to = emailMask
2648-
await fetch(concatLink(mailURL, '/send'), {
2671+
const response = await fetch(concatLink(mailURL, '/send'), {
26492672
method: 'post',
26502673
headers: {
26512674
'Content-Type': 'application/json'
@@ -2657,7 +2680,16 @@ export async function resendInvite (
26572680
to
26582681
})
26592682
})
2660-
ctx.info('Invite resend and email sent', { email: emailMask, workspace: wsPromise.workspace, link })
2683+
if (response.ok) {
2684+
ctx.info('Invite resend and email sent', { email: emailMask, workspace: wsPromise.workspace, link })
2685+
} else {
2686+
ctx.error('Failed to send invite resend email', {
2687+
email: emailMask,
2688+
workspace: wsPromise.workspace,
2689+
link,
2690+
error: response.statusText
2691+
})
2692+
}
26612693
}
26622694

26632695
async function deactivatePersonAccount (

Diff for: ‎services/mail/pod-mail/package.json

+3
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@
5555
"dependencies": {
5656
"@aws-sdk/client-ses": "^3.738.0",
5757
"@types/nodemailer": "^6.4.17",
58+
"@hcengineering/analytics-service": "^0.6.0",
59+
"@hcengineering/core": "^0.6.32",
60+
"@hcengineering/server-core": "^0.6.1",
5861
"cors": "^2.8.5",
5962
"dotenv": "~16.0.0",
6063
"express": "^4.21.2",

Diff for: ‎services/mail/pod-mail/src/__tests__/main.test.ts

+19-10
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
//
1515

1616
import { Request, Response } from 'express'
17+
import { type MeasureContext } from '@hcengineering/core'
1718
import { MailClient } from '../mail'
1819
import { handleSendMail } from '../main'
1920

@@ -31,6 +32,7 @@ describe('handleSendMail', () => {
3132
let res: Response
3233
let sendMailMock: jest.Mock
3334
let mailClient: MailClient
35+
let mockCtx: MeasureContext
3436

3537
beforeEach(() => {
3638
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
@@ -49,12 +51,16 @@ describe('handleSendMail', () => {
4951

5052
mailClient = new MailClient()
5153
sendMailMock = (mailClient.sendMessage as jest.Mock).mockResolvedValue({})
54+
mockCtx = {
55+
info: jest.fn(),
56+
error: jest.fn()
57+
} as unknown as MeasureContext
5258
})
5359

5460
it('should return 400 if text is missing', async () => {
5561
req.body.text = undefined
5662

57-
await handleSendMail(new MailClient(), req, res)
63+
await handleSendMail(new MailClient(), req, res, mockCtx)
5864

5965
// eslint-disable-next-line @typescript-eslint/unbound-method
6066
expect(res.status).toHaveBeenCalledWith(400)
@@ -64,7 +70,7 @@ describe('handleSendMail', () => {
6470
it('should return 400 if subject is missing', async () => {
6571
req.body.subject = undefined
6672

67-
await handleSendMail(new MailClient(), req, res)
73+
await handleSendMail(new MailClient(), req, res, mockCtx)
6874

6975
// eslint-disable-next-line @typescript-eslint/unbound-method
7076
expect(res.status).toHaveBeenCalledWith(400)
@@ -74,7 +80,7 @@ describe('handleSendMail', () => {
7480
it('should return 400 if to is missing', async () => {
7581
req.body.to = undefined
7682

77-
await handleSendMail(new MailClient(), req, res)
83+
await handleSendMail(new MailClient(), req, res, mockCtx)
7884

7985
// eslint-disable-next-line @typescript-eslint/unbound-method
8086
expect(res.status).toHaveBeenCalledWith(400)
@@ -84,49 +90,52 @@ describe('handleSendMail', () => {
8490
it('handles errors thrown by MailClient', async () => {
8591
sendMailMock.mockRejectedValue(new Error('Email service error'))
8692

87-
await handleSendMail(new MailClient(), req, res)
93+
await handleSendMail(new MailClient(), req, res, mockCtx)
8894

8995
expect(res.send).toHaveBeenCalled() // Check that a response is still sent
9096
})
9197

9298
it('should use source from config if from is not provided', async () => {
93-
await handleSendMail(mailClient, req, res)
99+
await handleSendMail(mailClient, req, res, mockCtx)
94100

95101
expect(sendMailMock).toHaveBeenCalledWith(
96102
expect.objectContaining({
97103
from: '[email protected]', // Verify that the default source from config is used
98104
to: '[email protected]',
99105
subject: 'Test Subject',
100106
text: 'Hello, world!'
101-
})
107+
}),
108+
mockCtx
102109
)
103110
})
104111

105112
it('should use from if it is provided', async () => {
106113
req.body.from = '[email protected]'
107-
await handleSendMail(mailClient, req, res)
114+
await handleSendMail(mailClient, req, res, mockCtx)
108115

109116
expect(sendMailMock).toHaveBeenCalledWith(
110117
expect.objectContaining({
111118
from: '[email protected]', // Verify that the from is used
112119
to: '[email protected]',
113120
subject: 'Test Subject',
114121
text: 'Hello, world!'
115-
})
122+
}),
123+
mockCtx
116124
)
117125
})
118126

119127
it('should send to multiple addresses', async () => {
120128
req.body.to = ['[email protected]', '[email protected]']
121-
await handleSendMail(mailClient, req, res)
129+
await handleSendMail(mailClient, req, res, mockCtx)
122130

123131
expect(sendMailMock).toHaveBeenCalledWith(
124132
expect.objectContaining({
125133
from: '[email protected]',
126134
to: ['[email protected]', '[email protected]'], // Verify that multiple addresses are passed
127135
subject: 'Test Subject',
128136
text: 'Hello, world!'
129-
})
137+
}),
138+
mockCtx
130139
)
131140
})
132141
})

Diff for: ‎services/mail/pod-mail/src/mail.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414
//
1515
import { type SendMailOptions, type Transporter } from 'nodemailer'
16+
import { MeasureContext } from '@hcengineering/core'
1617

1718
import config from './config'
1819
import { getTransport } from './transport'
@@ -24,14 +25,13 @@ export class MailClient {
2425
this.transporter = getTransport(config)
2526
}
2627

27-
async sendMessage (message: SendMailOptions): Promise<void> {
28+
async sendMessage (message: SendMailOptions, ctx: MeasureContext): Promise<void> {
2829
this.transporter.sendMail(message, (err, info) => {
2930
const messageInfo = `(from: ${message.from as string}, to: ${message.to as string})`
3031
if (err !== null) {
31-
console.error(`Failed to send email ${messageInfo}: `, err.message)
32-
console.log('Failed message details: ', message)
32+
ctx.error(`Failed to send email ${messageInfo}: ${err.message}`)
3333
} else {
34-
console.log(`Email request ${messageInfo} sent: ${info?.response}`)
34+
ctx.info(`Email request ${messageInfo} sent: ${info?.response}`)
3535
}
3636
})
3737
}

Diff for: ‎services/mail/pod-mail/src/main.ts

+32-10
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,39 @@
1616
import { type SendMailOptions } from 'nodemailer'
1717
import { Request, Response } from 'express'
1818
import Mail from 'nodemailer/lib/mailer'
19+
import { initStatisticsContext } from '@hcengineering/server-core'
20+
import { MeasureContext, MeasureMetricsContext, newMetrics } from '@hcengineering/core'
21+
import { SplitLogger } from '@hcengineering/analytics-service'
22+
import { join } from 'path'
1923

2024
import config from './config'
2125
import { createServer, listen } from './server'
2226
import { MailClient } from './mail'
2327
import { Endpoint } from './types'
2428

2529
export const main = async (): Promise<void> => {
30+
const measureCtx = initStatisticsContext('mail', {
31+
factory: () =>
32+
new MeasureMetricsContext(
33+
'mail',
34+
{},
35+
{},
36+
newMetrics(),
37+
new SplitLogger('mail', {
38+
root: join(process.cwd(), 'logs'),
39+
enableConsole: (process.env.ENABLE_CONSOLE ?? 'true') === 'true'
40+
})
41+
)
42+
})
2643
const client = new MailClient()
27-
console.log('Mail service has been started')
44+
measureCtx.info('Mail service has been started')
2845

2946
const endpoints: Endpoint[] = [
3047
{
3148
endpoint: '/send',
3249
type: 'post',
3350
handler: async (req, res) => {
34-
await handleSendMail(client, req, res)
51+
await handleSendMail(client, req, res, measureCtx)
3552
}
3653
}
3754
]
@@ -46,15 +63,20 @@ export const main = async (): Promise<void> => {
4663

4764
process.on('SIGINT', shutdown)
4865
process.on('SIGTERM', shutdown)
49-
process.on('uncaughtException', (e) => {
50-
console.error(e)
66+
process.on('uncaughtException', (e: any) => {
67+
measureCtx.error(e.message)
5168
})
52-
process.on('unhandledRejection', (e) => {
53-
console.error(e)
69+
process.on('unhandledRejection', (e: any) => {
70+
measureCtx.error(e.message)
5471
})
5572
}
5673

57-
export async function handleSendMail (client: MailClient, req: Request, res: Response): Promise<void> {
74+
export async function handleSendMail (
75+
client: MailClient,
76+
req: Request,
77+
res: Response,
78+
ctx: MeasureContext
79+
): Promise<void> {
5880
// Skip auth check, since service should be internal
5981
const { from, to, subject, text, html, attachments } = req.body
6082
const fromAddress = from ?? config.source
@@ -87,9 +109,9 @@ export async function handleSendMail (client: MailClient, req: Request, res: Res
87109
message.attachments = getAttachments(attachments)
88110
}
89111
try {
90-
await client.sendMessage(message)
91-
} catch (err) {
92-
console.log(err)
112+
await client.sendMessage(message, ctx)
113+
} catch (err: any) {
114+
ctx.error(err.message)
93115
}
94116

95117
res.send()

0 commit comments

Comments
 (0)