Skip to content

Commit 95faa6e

Browse files
authored
chore: dedupe public API conflict alerts (CM-1349) (#4414)
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
1 parent e8081d8 commit 95faa6e

9 files changed

Lines changed: 160 additions & 39 deletions

File tree

backend/.eslintrc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ module.exports = {
4747
},
4848
rules: {
4949
semi: ['error', 'never'],
50+
'no-void': 'off',
5051
'prefer-destructuring': ['error', { object: false, array: false }],
5152
'no-param-reassign': 0,
5253
'no-underscore-dangle': 0,
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import type { Request } from 'express'
2+
3+
import { ConflictError } from '@crowd/common'
4+
import type { IMemberIdentity } from '@crowd/types'
5+
6+
import { notifyOnce } from '@/api/public/alerts/notifyOnce'
7+
import { rethrowDbConflict } from '@/utils/err'
8+
9+
type IdentityConflictSubject = Pick<IMemberIdentity, 'memberId' | 'platform' | 'value' | 'type'>
10+
11+
function notifyIdentityConflict(
12+
req: Request,
13+
identity: IdentityConflictSubject,
14+
message: string,
15+
): void {
16+
const dedupeKey = [
17+
'member-identity-conflict',
18+
identity.platform,
19+
identity.type,
20+
identity.value,
21+
identity.memberId,
22+
]
23+
.filter(Boolean)
24+
.join(':')
25+
26+
void notifyOnce(req, dedupeKey, 'Identity conflict', [
27+
{
28+
title: 'Identity',
29+
text: `*Platform:* \`${identity.platform}\`\n*Type:* \`${identity.type}\`\n*Value:* \`${identity.value}\``,
30+
},
31+
...(identity.memberId
32+
? [{ title: 'Member', text: `*Member ID:* \`${identity.memberId}\`` }]
33+
: []),
34+
{ title: 'Conflict', text: `*Message:* ${message}` },
35+
{
36+
title: 'Request',
37+
text: `*Method:* \`${req.method}\`\n*URL:* \`${req.url}\``,
38+
},
39+
])
40+
}
41+
42+
/** Maps identity unique violations to ConflictError and alerts once. */
43+
export function rethrowIdentityConflict(
44+
req: Request,
45+
error: unknown,
46+
identity: IdentityConflictSubject,
47+
): never {
48+
try {
49+
rethrowDbConflict(error, {
50+
platform: identity.platform,
51+
value: identity.value,
52+
type: identity.type,
53+
})
54+
} catch (e) {
55+
if (e instanceof ConflictError) {
56+
notifyIdentityConflict(req, identity, e.message)
57+
}
58+
throw e
59+
}
60+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import type { Request } from 'express'
2+
3+
import { ConflictError } from '@crowd/common'
4+
5+
import { notifyOnce } from '@/api/public/alerts/notifyOnce'
6+
7+
function notifyMemberResolveConflict(req: Request, memberIds: string[], message: string): void {
8+
const dedupeKey = `member-resolve:${[...memberIds].sort().join(':')}`
9+
10+
void notifyOnce(req, dedupeKey, 'Member resolve conflict', [
11+
{
12+
title: 'Members',
13+
text: memberIds.map((id) => `• \`${id}\``).join('\n'),
14+
},
15+
{ title: 'Conflict', text: `*Message:* ${message}` },
16+
{
17+
title: 'Request',
18+
text: `*Method:* \`${req.method}\`\n*URL:* \`${req.url}\``,
19+
},
20+
])
21+
}
22+
23+
/** Throws ConflictError for ambiguous resolve and alerts once. */
24+
export function throwMemberResolveConflict(req: Request, memberIds: string[]): never {
25+
const message = 'Multiple member profiles matched'
26+
notifyMemberResolveConflict(req, memberIds, message)
27+
throw new ConflictError(message, { memberIds })
28+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import type { Request } from 'express'
2+
3+
import { generateUUIDv4 } from '@crowd/common'
4+
import { RedisCache } from '@crowd/redis'
5+
import {
6+
SlackChannel,
7+
type SlackMessageSection,
8+
SlackPersona,
9+
sendSlackNotification,
10+
} from '@crowd/slack'
11+
12+
/** Sends a Slack alert once per dedupe key. */
13+
export async function notifyOnce(
14+
req: Request,
15+
key: string,
16+
title: string,
17+
sections: SlackMessageSection[],
18+
): Promise<void> {
19+
const cache = new RedisCache('public-api-alerts', req.redis, req.log)
20+
const token = generateUUIDv4()
21+
22+
try {
23+
const result = await cache.setIfNotExistsOrGet(key, token, 60 * 60)
24+
25+
if (result !== token) {
26+
req.log.info({ key }, 'Skipping duplicate public API alert')
27+
return
28+
}
29+
} catch (err) {
30+
req.log.warn({ err, key }, 'Failed to deduplicate, sending alert')
31+
}
32+
33+
sendSlackNotification(
34+
SlackChannel.CDP_LFX_SELF_SERVE_ALERTS,
35+
SlackPersona.WARNING_PROPAGATOR,
36+
title,
37+
sections,
38+
)
39+
}

backend/src/api/public/middlewares/errorHandler.ts

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,7 @@ import {
44
UnauthorizedError as Auth0UnauthorizedError,
55
} from 'express-oauth2-jwt-bearer'
66

7-
import {
8-
ConflictError,
9-
HttpError,
10-
InsufficientScopeError,
11-
InternalError,
12-
UnauthorizedError,
13-
} from '@crowd/common'
7+
import { HttpError, InsufficientScopeError, InternalError, UnauthorizedError } from '@crowd/common'
148
import { SlackChannel, SlackPersona, sendSlackNotification } from '@crowd/slack'
159

1610
/**
@@ -23,30 +17,6 @@ export const errorHandler: ErrorRequestHandler = (
2317
res: Response,
2418
_next: NextFunction,
2519
) => {
26-
if (error instanceof ConflictError) {
27-
req.log.warn({ context: error.context }, 'Public API conflict')
28-
sendSlackNotification(
29-
SlackChannel.CDP_LFX_SELF_SERVE_ALERTS,
30-
SlackPersona.WARNING_PROPAGATOR,
31-
`Public API Conflict 409: ${req.method} ${req.url}`,
32-
[
33-
{
34-
title: 'Request',
35-
text: `*Method:* \`${req.method}\`\n*URL:* \`${req.url}\``,
36-
},
37-
{
38-
title: 'Conflict',
39-
text: `*Message:* ${error.message}`,
40-
},
41-
...(error.context
42-
? [{ title: 'Context', text: `\`\`\`${JSON.stringify(error.context, null, 2)}\`\`\`` }]
43-
: []),
44-
],
45-
)
46-
res.status(error.status).json(error.toJSON())
47-
return
48-
}
49-
5020
if (error instanceof HttpError) {
5121
res.status(error.status).json(error.toJSON())
5222
return

backend/src/api/public/v1/members/createMember.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getProperDisplayName } from '@crowd/common'
66
import { createMember as insertMember, insertMemberIdentities } from '@crowd/data-access-layer'
77
import { MemberIdentityType } from '@crowd/types'
88

9+
import { rethrowIdentityConflict } from '@/api/public/alerts/identityConflict'
910
import { optionsQx } from '@/database/sequelizeQueryExecutor'
1011
import { created } from '@/utils/api'
1112
import { rethrowDbConflict } from '@/utils/err'
@@ -61,6 +62,19 @@ export async function createMember(req: Request, res: Response): Promise<void> {
6162

6263
return { dbMember, dbIdentities }
6364
} catch (error) {
65+
// Only notify for a single identity because we can't tell which one conflicted in a batch.
66+
if (identities.length === 1) {
67+
const identity = identities[0]
68+
rethrowIdentityConflict(req, error, {
69+
platform: identity.platform,
70+
value:
71+
identity.type === MemberIdentityType.EMAIL
72+
? identity.value.trim().toLowerCase()
73+
: identity.value.trim(),
74+
type: identity.type,
75+
})
76+
}
77+
6478
return rethrowDbConflict(error)
6579
}
6680
})

backend/src/api/public/v1/members/identities/createMemberIdentity.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ import {
1313
} from '@crowd/data-access-layer'
1414
import { IMemberIdentity, MemberIdentityType } from '@crowd/types'
1515

16+
import { rethrowIdentityConflict } from '@/api/public/alerts/identityConflict'
1617
import { optionsQx } from '@/database/sequelizeQueryExecutor'
1718
import { created, ok } from '@/utils/api'
18-
import { rethrowDbConflict } from '@/utils/err'
1919
import { validateOrThrow } from '@/utils/validation'
2020

2121
const paramsSchema = z.object({
@@ -101,8 +101,12 @@ export async function createMemberIdentity(req: Request, res: Response): Promise
101101
}
102102
}
103103
} catch (error) {
104-
const ctx = { platform: data.platform, value: data.value, type: data.type }
105-
rethrowDbConflict(error, ctx)
104+
rethrowIdentityConflict(req, error, {
105+
memberId,
106+
platform: data.platform,
107+
value: data.value,
108+
type: data.type,
109+
})
106110
}
107111

108112
await touchMemberUpdatedAt(tx, memberId)

backend/src/api/public/v1/members/identities/verifyMemberIdentity.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ import {
2929
MemberUnmergeResult,
3030
} from '@crowd/types'
3131

32+
import { rethrowIdentityConflict } from '@/api/public/alerts/identityConflict'
3233
import { optionsQx } from '@/database/sequelizeQueryExecutor'
3334
import { noContent, ok } from '@/utils/api'
34-
import { rethrowDbConflict } from '@/utils/err'
3535
import { validateOrThrow } from '@/utils/validation'
3636

3737
const paramsSchema = z.object({
@@ -94,8 +94,12 @@ export async function verifyMemberIdentity(req: Request, res: Response): Promise
9494
})
9595
} catch (error) {
9696
if (verified) {
97-
const ctx = { platform: identity.platform, value: identity.value, type: identity.type }
98-
rethrowDbConflict(error, ctx)
97+
rethrowIdentityConflict(req, error, {
98+
memberId,
99+
platform: identity.platform,
100+
value: identity.value,
101+
type: identity.type,
102+
})
99103
}
100104

101105
throw error

backend/src/api/public/v1/members/resolveMember.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import type { Request, Response } from 'express'
22
import { z } from 'zod'
33

4-
import { ConflictError, NotFoundError } from '@crowd/common'
4+
import { NotFoundError } from '@crowd/common'
55
import { findMemberIdsByIdentities } from '@crowd/data-access-layer'
66
import { IMemberIdentity, MemberIdentityType, PlatformType } from '@crowd/types'
77

8+
import { throwMemberResolveConflict } from '@/api/public/alerts/memberResolveConflict'
89
import { optionsQx } from '@/database/sequelizeQueryExecutor'
910
import { ok } from '@/utils/api'
1011
import { validateOrThrow } from '@/utils/validation'
@@ -38,7 +39,7 @@ export async function resolveMemberByIdentities(req: Request, res: Response): Pr
3839
if (memberIds.length === 0) {
3940
throw new NotFoundError('Member not found')
4041
} else if (memberIds.length > 1) {
41-
throw new ConflictError('Multiple member profiles matched', { memberIds })
42+
throwMemberResolveConflict(req, memberIds)
4243
}
4344

4445
const memberId = memberIds[0]

0 commit comments

Comments
 (0)