Skip to content

Commit 8be62ad

Browse files
author
Dr.J
committed
Address PR review: add types, populate body/sent, and wrap request in try/catch
- Collapse duplicate 200/207 status blocks into single OR condition - Rename TrackApiResponse to CustomerIOBatchResponse and add as request generic - Add return type to getResponseBody - Thread original payloads and built batch through to populate body/sent in multi-status responses - Wrap batch request in try/catch to handle full payload failures
1 parent a36f1ab commit 8be62ad

File tree

2 files changed

+91
-55
lines changed

2 files changed

+91
-55
lines changed

packages/destination-actions/src/destinations/customerio/__tests__/utils.test.ts

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { MultiStatusResponse } from '@segment/actions-core'
2-
import { isIsoDate, parseTrackApiErrors, parseTrackApiMultiStatusResponse, resolveIdentifiers, sendBatch } from '../utils'
2+
import {
3+
isIsoDate,
4+
parseTrackApiErrors,
5+
parseTrackApiMultiStatusResponse,
6+
resolveIdentifiers,
7+
sendBatch
8+
} from '../utils'
39

410
describe('isIsoDate', () => {
511
it('should return true for valid ISO date with fractional seconds from 1-9 digits', () => {
@@ -115,18 +121,15 @@ describe('sendBatch', () => {
115121
expect((response as MultiStatusResponse).length()).toBe(2)
116122
expect((response as MultiStatusResponse).getResponseAtIndex(0).value()).toEqual({
117123
status: 200,
118-
body: {},
119-
sent: {}
124+
body: { person_id: 'user-1', name: 'First' },
125+
sent: { type: 'person', action: 'event', identifiers: { id: 'user-1' }, name: 'First' }
120126
})
121127
expect((response as MultiStatusResponse).getResponseAtIndex(1).value()).toEqual({
122128
status: 400,
123129
errormessage: 'Attribute value too long',
124130
errortype: 'PAYLOAD_VALIDATION_FAILED',
125-
body: {
126-
batch_index: 1,
127-
reason: 'invalid',
128-
message: 'Attribute value too long'
129-
}
131+
body: { person_id: 'user-2', name: 'Second' },
132+
sent: { type: 'person', action: 'event', identifiers: { id: 'user-2' }, name: 'Second' }
130133
})
131134
})
132135

@@ -159,18 +162,25 @@ describe('sendBatch', () => {
159162
status: 400,
160163
errormessage: 'Name is required',
161164
errortype: 'PAYLOAD_VALIDATION_FAILED',
162-
body: {
163-
batch_index: 0,
164-
reason: 'required',
165-
field: 'name',
166-
message: 'Name is required'
167-
}
165+
body: { person_id: 'user-1', name: 'First' },
166+
sent: { type: 'person', action: 'event', identifiers: { id: 'user-1' }, name: 'First' }
168167
})
169168
})
170169
})
171170

172171
describe('parseTrackApiErrors', () => {
173172
it('should fill success entries for items without errors', () => {
173+
const options = [
174+
{ type: 'person', action: 'event', settings: {}, payload: { person_id: 'user-0' } },
175+
{ type: 'person', action: 'event', settings: {}, payload: { person_id: 'user-1' } },
176+
{ type: 'person', action: 'event', settings: {}, payload: { person_id: 'user-2' } }
177+
]
178+
const batch = [
179+
{ type: 'person', action: 'event', identifiers: { id: 'user-0' } },
180+
{ type: 'person', action: 'event', identifiers: { id: 'user-1' } },
181+
{ type: 'person', action: 'event', identifiers: { id: 'user-2' } }
182+
]
183+
174184
const response = parseTrackApiErrors(
175185
[
176186
{
@@ -180,29 +190,36 @@ describe('parseTrackApiErrors', () => {
180190
message: 'Name is required'
181191
}
182192
],
183-
3
193+
options,
194+
batch
184195
)
185196

186197
expect(response.getAllResponses().map((result) => result.value())).toEqual([
187-
{ status: 200, body: {}, sent: {} },
198+
{
199+
status: 200,
200+
body: { person_id: 'user-0' },
201+
sent: { type: 'person', action: 'event', identifiers: { id: 'user-0' } }
202+
},
188203
{
189204
status: 400,
190205
errormessage: 'Name is required',
191206
errortype: 'PAYLOAD_VALIDATION_FAILED',
192-
body: {
193-
batch_index: 1,
194-
reason: 'required',
195-
field: 'name',
196-
message: 'Name is required'
197-
}
207+
body: { person_id: 'user-1' },
208+
sent: { type: 'person', action: 'event', identifiers: { id: 'user-1' } }
198209
},
199-
{ status: 200, body: {}, sent: {} }
210+
{
211+
status: 200,
212+
body: { person_id: 'user-2' },
213+
sent: { type: 'person', action: 'event', identifiers: { id: 'user-2' } }
214+
}
200215
])
201216
})
202217
})
203218

204219
describe('parseTrackApiMultiStatusResponse', () => {
205220
it('should return null for non-Track API response bodies', () => {
206-
expect(parseTrackApiMultiStatusResponse({ ok: true }, 1)).toBeNull()
221+
const options = [{ type: 'person', action: 'event', settings: {}, payload: { person_id: 'user-0' } }]
222+
const batch = [{ type: 'person', action: 'event', identifiers: { id: 'user-0' } }]
223+
expect(parseTrackApiMultiStatusResponse({ ok: true }, options, batch)).toBeNull()
207224
})
208225
})

packages/destination-actions/src/destinations/customerio/utils.ts

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import dayjs from '../../lib/dayjs'
22
import isPlainObject from 'lodash/isPlainObject'
33
import { fullFormats } from 'ajv-formats/dist/formats'
4-
import { ErrorCodes, MultiStatusResponse, type RequestClient } from '@segment/actions-core'
4+
import { ErrorCodes, MultiStatusResponse, RequestClient } from '@segment/actions-core'
55
import { CUSTOMERIO_TRACK_API_VERSION } from './versioning-info'
66

77
const isEmail = (value: string): boolean => {
@@ -208,30 +208,43 @@ export const sendBatch = async <Payload extends BasePayload>(
208208
const [{ settings }] = options
209209
const batch = options.map((opts) => buildPayload(opts))
210210

211-
const response = await request(`${trackApiEndpoint(settings)}/api/${CUSTOMERIO_TRACK_API_VERSION}/batch`, {
212-
method: 'post',
213-
json: {
214-
batch
215-
}
216-
})
211+
try {
212+
const response = await request<CustomerIOBatchResponse>(
213+
`${trackApiEndpoint(settings)}/api/${CUSTOMERIO_TRACK_API_VERSION}/batch`,
214+
{
215+
method: 'post',
216+
json: {
217+
batch
218+
}
219+
}
220+
)
217221

218-
const responseBody = getResponseBody(response)
222+
const responseBody = getResponseBody(response)
219223

220-
if (response?.status === 207 && responseBody) {
221-
const parsedResults = parseTrackApiMultiStatusResponse(responseBody, batch.length)
222-
if (parsedResults) {
223-
return parsedResults
224+
if ((response?.status === 200 || response?.status === 207) && responseBody) {
225+
const parsedResults = parseTrackApiMultiStatusResponse(responseBody, options, batch)
226+
if (parsedResults) {
227+
return parsedResults
228+
}
224229
}
225-
}
226230

227-
if (response?.status === 200 && responseBody) {
228-
const parsedResults = parseTrackApiMultiStatusResponse(responseBody, batch.length)
229-
if (parsedResults) {
230-
return parsedResults
231+
return response
232+
} catch (err) {
233+
const error = err as { message?: string; response?: { status?: number } }
234+
const status = error.response?.status ?? 500
235+
const message = error.message ?? 'Unknown error'
236+
237+
const multiStatusResponse = new MultiStatusResponse()
238+
for (let i = 0; i < options.length; i++) {
239+
multiStatusResponse.setErrorResponseAtIndex(i, {
240+
status,
241+
errormessage: message,
242+
body: options[i].payload,
243+
sent: batch[i]
244+
})
231245
}
246+
return multiStatusResponse
232247
}
233-
234-
return response
235248
}
236249

237250
interface TrackApiError {
@@ -241,7 +254,7 @@ interface TrackApiError {
241254
message?: string
242255
}
243256

244-
interface TrackApiResponse {
257+
interface CustomerIOBatchResponse {
245258
errors?: TrackApiError[]
246259
}
247260

@@ -266,7 +279,7 @@ function mapTrackApiReasonToErrorCode(reason: string | undefined) {
266279
}
267280
}
268281

269-
function getResponseBody(response: RequestResponse): unknown {
282+
function getResponseBody(response: RequestResponse): CustomerIOBatchResponse | string | undefined {
270283
const body = response.data ?? response.content ?? response.body
271284

272285
if (typeof body !== 'string') {
@@ -285,7 +298,11 @@ function getResponseBody(response: RequestResponse): unknown {
285298
}
286299
}
287300

288-
export function parseTrackApiErrors(errors: TrackApiError[], totalItems: number): MultiStatusResponse {
301+
export function parseTrackApiErrors<Payload extends BasePayload>(
302+
errors: TrackApiError[],
303+
options: RequestPayload<Payload>[],
304+
batch: Record<string, unknown>[]
305+
): MultiStatusResponse {
289306
const multiStatusResponse = new MultiStatusResponse()
290307
const errorMap = new Map<number, TrackApiError>()
291308

@@ -295,14 +312,14 @@ export function parseTrackApiErrors(errors: TrackApiError[], totalItems: number)
295312
}
296313
}
297314

298-
for (let i = 0; i < totalItems; i++) {
315+
for (let i = 0; i < options.length; i++) {
299316
const error = errorMap.get(i)
300317

301318
if (!error) {
302319
multiStatusResponse.setSuccessResponseAtIndex(i, {
303320
status: 200,
304-
body: {},
305-
sent: {}
321+
body: options[i].payload,
322+
sent: batch[i]
306323
})
307324
continue
308325
}
@@ -311,27 +328,29 @@ export function parseTrackApiErrors(errors: TrackApiError[], totalItems: number)
311328
status: 400,
312329
errormessage: error.message || `${error.reason || 'ERROR'}: ${error.field || 'unknown field'}`,
313330
errortype: mapTrackApiReasonToErrorCode(error.reason),
314-
body: error
331+
body: options[i].payload,
332+
sent: batch[i]
315333
})
316334
}
317335

318336
return multiStatusResponse
319337
}
320338

321-
export function parseTrackApiMultiStatusResponse(
322-
responseBody: unknown,
323-
totalItems: number
339+
export function parseTrackApiMultiStatusResponse<Payload extends BasePayload>(
340+
responseBody: CustomerIOBatchResponse | string | undefined,
341+
options: RequestPayload<Payload>[],
342+
batch: Record<string, unknown>[]
324343
): MultiStatusResponse | null {
325344
if (!isRecord(responseBody)) {
326345
return null
327346
}
328347

329-
const { errors } = responseBody as TrackApiResponse
348+
const { errors } = responseBody as CustomerIOBatchResponse
330349
if (!Array.isArray(errors) || errors.length === 0) {
331350
return null
332351
}
333352

334-
return parseTrackApiErrors(errors, totalItems)
353+
return parseTrackApiErrors(errors, options, batch)
335354
}
336355

337356
export const sendSingle = <Payload extends BasePayload>(request: RequestClient, options: RequestPayload<Payload>) => {

0 commit comments

Comments
 (0)