fix(google-enhanced-conversions): resolve partial failures against the request that failed

[peterdemartini]

In mirror mode a userList batch is submitted to Google as two separate addOperations requests — one carrying the create operations, one carrying the remove operations. Google reports a partial failure by the operation's index within the request that failed, but both requests were handed the same combined validPayloadIndicesBitmap, which covers every valid event of either type.

Those two index spaces coincide only when one of the arrays is empty. That is why syncMode: 'add' and syncMode: 'delete' are unaffected, and why the existing all-add partial-failure test passes.

In mirror mode the consequence is a wrong per-event status: a successfully delivered event is reported as failed, while the event Google actually rejected is reported as delivered. Counts stay correct, so nothing looks wrong in aggregate.

The line immediately below the mapping is the tell — sent: userIdentifiers?.[failedIndex] reads the submitted array and was already correct, so the response attached the right operation payload to the wrong event. The two fields were resolved against different arrays.

The same combined map was also used to fan out a whole-request failure in handleGoogleAdsAPIErrorResponse, so a remove request failing outright marked every added event failed too, including adds Google had accepted. Because parseGoogleAdsError maps CONCURRENT_MODIFICATION to a retryable 429, that could also send accepted adds back for retry.

Fix: extractBatchUserIdentifiers now also returns addPayloadIndices and removePayloadIndices, and each processOperations call is given the indices of the events it actually carried. The combined map stays where it is genuinely union-scoped — updateMultiStatusResponseWithSuccess and handleJobExecutionError.

Nothing outside this destination is affected: amazon-conversions-api uses a similar bitmap but submits a single request, so its map is correctly scoped already.

Behavior change

Per the repo's PR guidelines on altering existing behavior: for mirror-mode batches that hit a partial or whole-request failure, per-event statuses change — that is the fix. Events previously reported failed-but-delivered now report success, and vice versa. Add-only and delete-only sync modes are byte-identical.

Testing

• Added unit tests for new functionality
• Tested end-to-end using the local server
• [If destination is already live] Tested for backward compatibility of destination
• [Segmenters] Tested in the staging environment
• [Segmenters] [If applicable for this change] Tested for regression with Hadron

Two regression tests, both verified to fail on main and pass with the fix. The existing suite only covered the all-add shape, which cannot catch this class of bug.

1. attributes a partial failure on the remove request to the event that actually failed — three events in mirror mode (one add, two removes); the remove request returns a partial failure at operations index 1, which is batch index 2.

On main:

✕ attributes a partial failure on the remove request to the event that actually failed
  -   "status": 200,
  +   "status": 400,
  > expect(responses[1]).toMatchObject({ status: 200 })

The error lands on responses[1] — the removed event that succeeded — instead of responses[2].

2. does not fail the added events when only the remove request fails — same batch shape; the remove request fails outright. On main the added event at index 0 is marked failed; with the fix it stays 200.

yarn cloud jest --testPathPattern="google-enhanced-conversions"
Test Suites: 12 passed, 12 total
Tests:       215 passed, 215 total
Snapshots:   121 passed, 121 total

Prettier and ESLint clean on both changed files (ESLint reports only pre-existing any-related warnings, 0 errors).

Note on local hooks: the pre-commit hook could not run here — lint-staged rejects this machine's Node 26 against the repo's ^18.17 || ^22.13 engine constraint — so it was committed with --no-verify after running its checks by hand: prettier --check (clean), eslint (0 errors), the destination's full jest suite (215 passing), and gitleaks protect --staged (no leaks). Worth a second look from CI.

Security Review

• Reviewed all field definitions for sensitive data — no field definitions changed; this touches only per-event status attribution.

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes incorrect per-event status attribution in Google Enhanced Conversions mirror mode by scoping failure indices to the specific add/remove request that failed, and adds regression coverage for both partial and whole-request failures.

Changes:

• Track per-request payload indices (addPayloadIndices / removePayloadIndices) and use them when mapping Google operation indices back to original batch indices.
• Update whole-request and partial-failure handlers to resolve indices against the request-specific index list.
• Add unit tests covering mirror-mode remove partial failures and remove whole-request failures.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
packages/destination-actions/src/destinations/google-enhanced-conversions/functions.ts	Fixes index-space mismatch by passing request-scoped indices into error mapping logic.
packages/destination-actions/src/destinations/google-enhanced-conversions/tests/userList.test.ts	Adds regression tests proving correct per-event attribution for remove-request failures in mirror mode.

Suppressed comments (1)

packages/destination-actions/src/destinations/google-enhanced-conversions/functions.ts:787

• For whole-request failures, each event gets the same sent: payload (the entire addOperations request payload), which is less useful than attaching the specific operation corresponding to that event (and is inconsistent with partial-failure handling, which uses userIdentifiers?.[failedIndex]). Consider passing the per-request userIdentifiers array into handleGoogleAdsAPIErrorResponse and iterating as (originalIndex, requestIndex) => ... sent: userIdentifiers?.[requestIndex] (optionally keeping the full request payload under a different field if needed for debugging).

  const errorData = error?.response?.data?.error
  const parsedError = parseGoogleAdsError(errorData)
  operationPayloadIndices.forEach((index) => {
    multiStatusResponse.setErrorResponseAtIndex(index, {
      ...parsedError,
      body: error,
      sent: payload
    })

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[joe-ayoub-segment]

@peterdemartini I think this JIRA ticket is for the bug you are addressing here.
https://twilio-engineering.atlassian.net/browse/STRATCONN-6862

OK if I update the PR description to reference the ticket ID?

[peterdemartini]

@peterdemartini I think this JIRA ticket is for the bug you are addressing here. https://twilio-engineering.atlassian.net/browse/STRATCONN-6862

OK if I update the PR description to reference the ticket ID?

@joe-ayoub-segment absolutely, I won't really have time to test/deploy it. This is just something I/Claude found while investigating a count issue for a customer.

[joe-ayoub-segment]

@peterdemartini I think this JIRA ticket is for the bug you are addressing here. https://twilio-engineering.atlassian.net/browse/STRATCONN-6862
OK if I update the PR description to reference the ticket ID?

@joe-ayoub-segment absolutely, I won't really have time to test/deploy it. This is just something I/Claude found while investigating a count issue for a customer.

I'll take the PR through to testing -> Prod then. Not ETA yet though.