Skip to content

Commit a14dea4

Browse files
authored
fix: keep contacts from succeeded sources when an extractor fails [CM-1341] (#4362)
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
1 parent 9c0f754 commit a14dea4

4 files changed

Lines changed: 53 additions & 22 deletions

File tree

services/apps/packages_worker/src/security-contacts/ingestSingle.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,10 @@ export async function ingestSecurityContactsForPurl(
9494
const baseDeps = buildBaseDeps(config)
9595
try {
9696
const result = await processRepo(target, baseDeps, cdpQx)
97-
if (result.status === 'ok') {
98-
await writeContacts(qx, result.repoId, result.contacts, result.policies)
97+
if (result.status === 'ok' || result.status === 'partial') {
98+
await writeContacts(qx, result.repoId, result.contacts, result.policies, {
99+
merge: result.status === 'partial',
100+
})
99101
} else {
100102
await markRepoAttempted(qx, result.repoId)
101103
}

services/apps/packages_worker/src/security-contacts/processBatch.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,16 @@ export async function processRepo(
135135

136136
const results = await Promise.allSettled(EXTRACTORS.map((extract) => extract(target, deps)))
137137

138-
// Write only when every extractor succeeded, so a failed one can't wipe contacts it didn't see.
139-
const failed = results.find((r) => r.status === 'rejected') as PromiseRejectedResult | undefined
140-
if (failed) {
138+
let failedCount = 0
139+
results.forEach((r, i) => {
140+
if (r.status !== 'rejected') return
141+
failedCount++
141142
log.warn(
142-
{ repoId: target.repoId, errMsg: failed.reason?.message },
143-
'Extractor failed — preserving existing data',
143+
{ repoId: target.repoId, extractor: EXTRACTORS[i].name, errMsg: r.reason?.message },
144+
'Extractor failed — keeping contacts from remaining sources',
144145
)
146+
})
147+
if (failedCount === results.length) {
145148
return { repoId: target.repoId, status: 'extractor-failed' }
146149
}
147150

@@ -177,7 +180,12 @@ export async function processRepo(
177180
}
178181

179182
const scored = reconcile(contacts)
180-
return { repoId: target.repoId, status: 'ok', contacts: scored, policies }
183+
return {
184+
repoId: target.repoId,
185+
status: failedCount > 0 ? 'partial' : 'ok',
186+
contacts: scored,
187+
policies,
188+
}
181189
}
182190

183191
export async function processBatch(
@@ -228,10 +236,12 @@ export async function processBatch(
228236

229237
const extractionDurationMs = Date.now() - extractionStartedAt
230238
const ok = outcomes.filter((o) => o.status === 'ok').length
239+
const partial = outcomes.filter((o) => o.status === 'partial').length
231240
log.info(
232241
{
233242
ok,
234-
failed: outcomes.length - ok,
243+
partial,
244+
failed: outcomes.length - ok - partial,
235245
durationMs: extractionDurationMs,
236246
reposPerSec: Number((outcomes.length / (extractionDurationMs / 1000)).toFixed(1)),
237247
},

services/apps/packages_worker/src/security-contacts/types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,10 @@ export type Extractor = (target: RepoTarget, deps: ExtractorDeps) => Promise<Ext
8181

8282
export type ProcessRepoResult =
8383
| { repoId: string; status: 'ok'; contacts: ScoredContact[]; policies: Partial<RepoPolicies> }
84+
| {
85+
repoId: string
86+
status: 'partial'
87+
contacts: ScoredContact[]
88+
policies: Partial<RepoPolicies>
89+
}
8490
| { repoId: string; status: 'extractor-failed' }

services/apps/packages_worker/src/security-contacts/writeContacts.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,23 @@ export async function markRepoAttempted(qx: QueryExecutor, repoId: string): Prom
5757
//
5858
// Soft-delete: mark every active row stale, then upsert this pass's contacts, reviving whatever
5959
// was rediscovered. Readers of this table must filter on deleted_at IS NULL.
60+
//
61+
// merge: skip the soft-delete when this pass ran with a failed extractor — a failed source
62+
// can't wipe contacts it didn't see; stale rows are cleaned on the next fully-successful pass.
6063
export async function writeContacts(
6164
qx: QueryExecutor,
6265
repoId: string,
6366
contacts: ScoredContact[],
6467
policies: Partial<RepoPolicies>,
68+
opts: { merge?: boolean } = {},
6569
): Promise<void> {
6670
await qx.tx(async (tx) => {
67-
await tx.result(
68-
'UPDATE security_contacts SET deleted_at = NOW(), updated_at = NOW() WHERE repo_id = $(repoId) AND deleted_at IS NULL',
69-
{ repoId },
70-
)
71+
if (!opts.merge) {
72+
await tx.result(
73+
'UPDATE security_contacts SET deleted_at = NOW(), updated_at = NOW() WHERE repo_id = $(repoId) AND deleted_at IS NULL',
74+
{ repoId },
75+
)
76+
}
7177

7278
if (contacts.length > 0) {
7379
await tx.result(
@@ -114,12 +120,12 @@ export async function markReposAttempted(qx: QueryExecutor, repoIds: string[]):
114120
)
115121
}
116122

117-
type OkResult = Extract<ProcessRepoResult, { status: 'ok' }>
123+
type PersistableResult = Extract<ProcessRepoResult, { status: 'ok' | 'partial' }>
118124

119125
// Bounds blast radius: a bad row only forces a re-extract of its chunk next sweep, not the whole batch.
120126
const WRITE_CHUNK_SIZE = 100
121127

122-
function prepareBulkPolicyUpdate(chunk: OkResult[]): string {
128+
function prepareBulkPolicyUpdate(chunk: PersistableResult[]): string {
123129
const rows = chunk.map(
124130
(_, i) =>
125131
`($(id${i})::bigint, $(securityPolicyUrl${i}), $(vulnerabilityReportingUrl${i}), $(pvrResolved${i})::boolean, $(bugBountyUrl${i}), $(securityTxtUrl${i}), $(pvrEnabled${i})::boolean)`,
@@ -156,15 +162,20 @@ function prepareBulkPolicyUpdate(chunk: OkResult[]): string {
156162
)
157163
}
158164

159-
async function writeContactsChunk(qx: QueryExecutor, chunk: OkResult[]): Promise<void> {
165+
async function writeContactsChunk(qx: QueryExecutor, chunk: PersistableResult[]): Promise<void> {
160166
if (chunk.length === 0) return
161-
const repoIds = chunk.map((r) => r.repoId)
167+
168+
// Partial repos get a merge-only write: their failed extractor couldn't re-see existing
169+
// contacts, so soft-deleting would wipe data the pass never evaluated.
170+
const fullRefreshIds = chunk.filter((r) => r.status === 'ok').map((r) => r.repoId)
162171

163172
await qx.tx(async (tx) => {
164-
await tx.result(
165-
'UPDATE security_contacts SET deleted_at = NOW(), updated_at = NOW() WHERE repo_id = ANY($(repoIds)::bigint[]) AND deleted_at IS NULL',
166-
{ repoIds },
167-
)
173+
if (fullRefreshIds.length > 0) {
174+
await tx.result(
175+
'UPDATE security_contacts SET deleted_at = NOW(), updated_at = NOW() WHERE repo_id = ANY($(repoIds)::bigint[]) AND deleted_at IS NULL',
176+
{ repoIds: fullRefreshIds },
177+
)
178+
}
168179

169180
const rows = chunk.flatMap((r) => r.contacts.map((c) => toContactRow(r.repoId, c)))
170181
if (rows.length > 0) {
@@ -184,7 +195,9 @@ export async function writeContactsBatch(
184195
const attemptedOnlyIds = outcomes
185196
.filter((o) => o.status === 'extractor-failed')
186197
.map((o) => o.repoId)
187-
const ok = outcomes.filter((o): o is OkResult => o.status === 'ok')
198+
const ok = outcomes.filter(
199+
(o): o is PersistableResult => o.status === 'ok' || o.status === 'partial',
200+
)
188201

189202
let firstError: Error | undefined
190203
for (let i = 0; i < ok.length; i += WRITE_CHUNK_SIZE) {

0 commit comments

Comments
 (0)