STRATCONN-6937 [Braze] Fix ecommerce batch error index mismatch and sent/body population - #3929
STRATCONN-6937 [Braze] Fix ecommerce batch error index mismatch and sent/body population#3929joe-ayoub-segment wants to merge 3 commits into
Conversation
…ent/body population
Batch error matching in ecommerce send() used the original loop index to
look up Braze's errors[].index, but Braze's index refers to the events
array actually sent (excluding payloads filtered by validate()). Match on
the sent-array index (payload.index) instead, while still recording the
MultiStatus response at the original payload position.
Also populate the MultiStatusResponse sent/body attributes correctly:
- Only set `sent` when JSON was actually sent, to the per-item JSON.
- Only set `body` when a response exists: { success: true } for 2XX,
error details for non-2XX.
- Omit both when a payload was never sent (filtered by validate() or an
invalid syncMode batch).
Shared by the ecommerce and ecommerceSingleProduct actions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 Braze ecommerce batch MultiStatus handling by correctly aligning Braze per-item errors to the sent events[] indices and by adjusting when/how sent and body are populated to improve error classification (pre-send vs destination).
Changes:
- Match Braze
errors[].indexagainst the index in the senteventsarray (payload.index) instead of the original payload loop index. - Populate MultiStatus
sent/bodyonly for payloads actually sent; use{ success: true }for successful items. - Update/extend batch multi-status tests to cover index mismatch, fully successful batches, and new
sent/bodyshapes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/destination-actions/src/destinations/braze/ecommerce/functions.ts | Fixes batch error attribution and revises MultiStatus sent/body population semantics. |
| packages/destination-actions/src/destinations/braze/ecommerce/tests/index.test.ts | Updates expectations and adds coverage for the corrected index mapping and new sent/body behavior. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…esponse.data Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/destination-actions/src/destinations/braze/ecommerce/tests/index.test.ts:1241
- In the invalid-syncMode batch test,
send()returns early and should not call/users/track. Leaving this nock interceptor in place can cause the test suite to fail if it enforces that all nocks are consumed. Remove this interceptor (or change the test to explicitly assert that no request was made).
const responseJSON = [invalidSyncModeError, invalidSyncModeError, invalidSyncModeError, invalidSyncModeError]
nock(settings.endpoint).post('/users/track', json).reply(200)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:69
- This does an
errors.find(...)for each payload (O(n*m)). Sinceerrors[].indexis a direct lookup key, consider building aMap<number, Error>once (e.g.,const errorByIndex = new Map(errors.map(e => [e.index, e]))) and then doingerrorByIndex.get(sentIndex)inside the loop.
const errors = Array.isArray(response.data?.errors) ? response.data.errors : []
payloadsWithIndexes.forEach((payload, index) => {
const sentIndex = payload.index
if (sentIndex === undefined) {
return
}
const error = errors.find((e) => e.index === sentIndex)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:78
errormessageandbodyare currently the same string (error.type), while successful items setbodyto an object ({ success: true }). To make the MultiStatus shape more consistent and more useful for debugging, consider settingbodyto a structured object (e.g., the full Braze error object) and keeperrormessageas the summary string.
msResponse.setErrorResponseAtIndex(index, {
status: 400,
errortype: 'BAD_REQUEST',
errormessage: error.type,
sent: json.events[sentIndex] as object as JSONLikeObject,
body: error.type
})
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (6)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:63
- The lookup
errors.find(...)inside thepayloadsWithIndexes.forEach(...)loop is O(n²) for batch sizes and also obscures intent. Consider pre-indexing Braze errors into aMap<number, Error>keyed byindex(sent-array index) and doing O(1) lookups per payload.
const errors = Array.isArray(response.data?.errors) ? response.data.errors : []
payloadsWithIndexes.forEach((payload, index) => {
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:69
- The lookup
errors.find(...)inside thepayloadsWithIndexes.forEach(...)loop is O(n²) for batch sizes and also obscures intent. Consider pre-indexing Braze errors into aMap<number, Error>keyed byindex(sent-array index) and doing O(1) lookups per payload.
const error = errors.find((e) => e.index === sentIndex)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:96
- This function mutates the incoming payload objects (
payload.index = ...) to track sent-array indices. Sinceindexis a fairly generic field name and payload objects can be reused/inspected later in the pipeline, this side-effect can be surprising and hard to reason about. Consider keeping the mapping in a separate structure (e.g.,sentIndexByOriginalIndex: Array<number | undefined>) or wrapping payloads in a new object{ payload, sentIndex }rather than mutating the payload itself.
payloadsWithIndexes.forEach((payload, index) => {
const message = validate(payload, isBatch)
if (message) {
payload.index = undefined
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:103
- This function mutates the incoming payload objects (
payload.index = ...) to track sent-array indices. Sinceindexis a fairly generic field name and payload objects can be reused/inspected later in the pipeline, this side-effect can be surprising and hard to reason about. Consider keeping the mapping in a separate structure (e.g.,sentIndexByOriginalIndex: Array<number | undefined>) or wrapping payloads in a new object{ payload, sentIndex }rather than mutating the payload itself.
const event = getJSONItem(payload, settings, syncMode)
payload.index = events.length
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:78
- For destination-side per-item failures,
bodyis currently set to the same string aserrormessage(error.type). This drops potentially useful diagnostic detail from the Braze response (e.g., the full error object including the index/type and any additional fields). Consider storing richer error details inbody(for example, the fullerrorobject or a structured{ error: ... }) so downstream debugging and observability have more context.
msResponse.setErrorResponseAtIndex(index, {
status: 400,
errortype: 'BAD_REQUEST',
errormessage: error.type,
sent: json.events[sentIndex] as object as JSONLikeObject,
body: error.type
})
packages/destination-actions/src/destinations/braze/ecommerce/tests/index.test.ts:1241
- In the invalid-syncMode batch test, the implementation now returns early without making a request, so this Nock interceptor is unused. Leaving unused interceptors can make tests brittle if the suite later enforces
nock.isDone()/no pending mocks. Consider removing this interceptor (or explicitly asserting no request was made, if that’s the intent).
nock(settings.endpoint).post('/users/track', json).reply(200)
Summary
Fixes STRATCONN-6937. Two related fixes in
braze/ecommerce/functions.tssend()(shared by both theecommerceandecommerceSingleProductactions):1. Batch error index mismatch
Batch-mode error matching used the original loop
indexto look up Braze'serrors[].index, but Braze's index refers to the position within theeventsarray actually sent (which excludes payloads filtered out byvalidate()). When an earlier payload failed validation, the indices diverged and Braze errors were attributed to the wrong payload.Now matches on the sent-array index (
payload.index, already tracked ingetJSON()), while still recording the MultiStatus response at the original payload position.2. MultiStatusResponse
sent/bodypopulationsent— only set when JSON was actually sent to Braze, populated with the per-item JSON that was sent. Omitted when nothing was sent.body— only set when a response exists:{ success: true }for 2XX, error details for non-2XX. Omitted when nothing was sent.validate(), or an invalid syncMode batch) now carry neithersentnorbody— so core correctly classifies them asINTEGRATIONS(pre-send validation) rather thanDESTINATIONerrors.Testing
sent/bodyshapes.sent/bodyon both the Braze-rejected and successful items.sent+{ success: true }.braze/ecommercetests pass; lint + typecheck clean.Requires stage test.
🤖 Generated with Claude Code