diff --git a/packages/destination-actions/src/destinations/google-enhanced-conversions/__tests__/userList.test.ts b/packages/destination-actions/src/destinations/google-enhanced-conversions/__tests__/userList.test.ts index 9b26f7cae6e..d2156321d3c 100644 --- a/packages/destination-actions/src/destinations/google-enhanced-conversions/__tests__/userList.test.ts +++ b/packages/destination-actions/src/destinations/google-enhanced-conversions/__tests__/userList.test.ts @@ -1170,6 +1170,138 @@ describe('GoogleEnhancedConversions', () => { }) }) + it('attributes a partial failure on the remove request to the event that actually failed', async () => { + // A mirror-mode batch is submitted as two separate addOperations calls, one + // carrying the create operations and one carrying the remove operations. + // Google reports a partial failure by its index within the request it + // failed in, so an index from the remove call resolves against the removed + // events only. + const events: SegmentEvent[] = [ + createTestEvent({ + timestamp, + event: 'new', + properties: { email: 'added@gmail.com' } + }), + createTestEvent({ + timestamp, + event: 'deleted', + properties: { email: 'removed-ok@gmail.com' } + }), + createTestEvent({ + timestamp, + event: 'deleted', + properties: { email: 'removed-bad@gmail.com' } + }) + ] + + nock(`https://googleads.googleapis.com/${API_VERSION}/customers/${customerId}/offlineUserDataJobs:create`) + .post(/.*/) + .reply(200, { resourceName: 'customers/1234/userLists/1234' }) + + // The create operations all succeed. + nock(`https://googleads.googleapis.com/${API_VERSION}/offlineDataJob:addOperations`).post(/.*/).reply(200, {}) + + // The second remove fails: operations index 1 of the remove request, which + // is event index 2 of the batch. + nock(`https://googleads.googleapis.com/${API_VERSION}/offlineDataJob:addOperations`) + .post(/.*/) + .reply(200, { + partialFailureError: { + code: 3, + message: 'Mocking Partial Failure Error', + details: [ + { + '@type': 'type.googleapis.com/google.ads.googleads.v21.errors.GoogleAdsFailure', + errors: [ + { + errorCode: { offlineUserDataJobError: 'INVALID_SHA256_FORMAT' }, + message: 'The SHA256 encoded value is malformed.', + location: { + fieldPathElements: [ + { fieldName: 'operations', index: 1 }, + { fieldName: 'remove' }, + { fieldName: 'user_identifiers', index: 0 }, + { fieldName: 'hashed_email' } + ] + } + } + ] + } + ] + } + }) + + nock(`https://googleads.googleapis.com/${API_VERSION}/offlineDataJob:run`).post(/.*/).reply(200, { done: true }) + + const responses = await testDestination.executeBatch('userList', { + events, + mapping: { ...mapping, __segment_internal_sync_mode: 'mirror' }, + settings: { customerId } + }) + + // The added event was never part of the failing request. + expect(responses[0]).toMatchObject({ status: 200 }) + // The first removed event succeeded. + expect(responses[1]).toMatchObject({ status: 200 }) + // The second removed event is the one Google rejected. + expect(responses[2]).toMatchObject({ + status: 400, + errormessage: 'The SHA256 encoded value is malformed.' + }) + }) + + it('does not fail the added events when only the remove request fails', async () => { + // A request failing outright says nothing about the events submitted in the + // other request. + const events: SegmentEvent[] = [ + createTestEvent({ + timestamp, + event: 'new', + properties: { email: 'added@gmail.com' } + }), + createTestEvent({ + timestamp, + event: 'deleted', + properties: { email: 'removed-ok@gmail.com' } + }), + createTestEvent({ + timestamp, + event: 'deleted', + properties: { email: 'removed-bad@gmail.com' } + }) + ] + + nock(`https://googleads.googleapis.com/${API_VERSION}/customers/${customerId}/offlineUserDataJobs:create`) + .post(/.*/) + .reply(200, { resourceName: 'customers/1234/userLists/1234' }) + + // The create operations succeed. + nock(`https://googleads.googleapis.com/${API_VERSION}/offlineDataJob:addOperations`).post(/.*/).reply(200, {}) + + // The remove request fails outright. + nock(`https://googleads.googleapis.com/${API_VERSION}/offlineDataJob:addOperations`) + .post(/.*/) + .reply(400, { + error: { + code: 400, + message: 'Remove operations rejected', + details: [] + } + }) + + nock(`https://googleads.googleapis.com/${API_VERSION}/offlineDataJob:run`).post(/.*/).reply(200, { done: true }) + + const responses = await testDestination.executeBatch('userList', { + events, + mapping: { ...mapping, __segment_internal_sync_mode: 'mirror' }, + settings: { customerId } + }) + + expect(responses[0]).toMatchObject({ status: 200 }) + expect(responses[1]).toMatchObject({ status: 400 }) + expect(responses[2]).toMatchObject({ status: 400 }) + }) + it('should successfully handle a Partial failure error from addOperation offlineUserDataJobs API', async () => { const events: SegmentEvent[] = [ // Assume this Payload gets failed in Partial Failure diff --git a/packages/destination-actions/src/destinations/google-enhanced-conversions/functions.ts b/packages/destination-actions/src/destinations/google-enhanced-conversions/functions.ts index 35706b931f0..a6487b0d90e 100644 --- a/packages/destination-actions/src/destinations/google-enhanced-conversions/functions.ts +++ b/packages/destination-actions/src/destinations/google-enhanced-conversions/functions.ts @@ -660,7 +660,9 @@ const processOperations = async ( request: RequestClient, userIdentifiers: any, resourceName: string, - validPayloadIndicesBitmap: number[], + // The original indices of the events carried by this request only, in the + // order they were submitted. + operationPayloadIndices: number[], failedPayloadIndices: Set, multiStatusResponse: MultiStatusResponse, features?: Features | undefined, @@ -671,7 +673,7 @@ const processOperations = async ( if (!success) { handleGoogleAdsAPIErrorResponse( error as GoogleAdsError, - validPayloadIndicesBitmap, + operationPayloadIndices, multiStatusResponse, operationPayload, failedPayloadIndices @@ -682,7 +684,7 @@ const processOperations = async ( if (partialFailureError) { handlePartialFailureResponse( partialFailureError, - validPayloadIndicesBitmap, + operationPayloadIndices, multiStatusResponse, userIdentifiers, failedPayloadIndices @@ -768,14 +770,16 @@ export const verifyCustomerId = (customerId: string | undefined) => { const handleGoogleAdsAPIErrorResponse = ( error: any, - validPayloadIndicesBitmap: number[], + // The original indices of the events the failed request carried. A request + // failing outright says nothing about events submitted in the other request. + operationPayloadIndices: number[], multiStatusResponse: MultiStatusResponse, payload: JSONLikeObject, failedPayloadIndices?: Set ) => { const errorData = error?.response?.data?.error const parsedError = parseGoogleAdsError(errorData) - validPayloadIndicesBitmap.forEach((index) => { + operationPayloadIndices.forEach((index) => { multiStatusResponse.setErrorResponseAtIndex(index, { ...parsedError, body: error, @@ -826,7 +830,8 @@ const updateMultiStatusResponseWithSuccess = ( export const handlePartialFailureResponse = ( partialFailureError: any, - validPayloadIndicesBitmap: number[], + // The original indices of the events carried by the failing request only. + operationPayloadIndices: number[], multiStatusResponse: MultiStatusResponse, userIdentifiers: any[], failedPayloadIndices: Set @@ -838,7 +843,7 @@ export const handlePartialFailureResponse = ( )?.index if (failedIndex >= 0) { - const originalIndex = validPayloadIndicesBitmap[failedIndex] + const originalIndex = operationPayloadIndices[failedIndex] multiStatusResponse.setErrorResponseAtIndex(originalIndex, { status: STATUS_CODE_MAPPING?.[partialFailureError.code as keyof typeof STATUS_CODE_MAPPING]?.status ?? 500, // error code errormessage: error.message, @@ -924,6 +929,13 @@ const extractBatchUserIdentifiers = ( const removeUserIdentifiers: any[] = [] const addUserIdentifiers: any[] = [] const validPayloadIndicesBitmap: number[] = [] + // The add and remove operations are submitted as two separate requests, and + // Google reports a failure by its index within the request it failed in. Each + // request therefore needs the indices of the events it actually carried; + // resolving through the combined map above lands on whichever valid event + // happens to sit at that position across both. + const addPayloadIndices: number[] = [] + const removePayloadIndices: number[] = [] //Identify the user identifiers based on the idType const extractors = createIdentifierExtractors(features) @@ -962,13 +974,21 @@ const extractBatchUserIdentifiers = ( validPayloadIndicesBitmap.push(index) if (operationType === 'add') { + addPayloadIndices.push(index) addUserIdentifiers.push({ create: { userIdentifiers } }) } else { + removePayloadIndices.push(index) removeUserIdentifiers.push({ remove: { userIdentifiers } }) } }) - return { addUserIdentifiers, removeUserIdentifiers, validPayloadIndicesBitmap } + return { + addUserIdentifiers, + removeUserIdentifiers, + validPayloadIndicesBitmap, + addPayloadIndices, + removePayloadIndices + } } // Helper function to determine operation type @@ -1024,14 +1044,13 @@ export const processBatchPayload = async ( const multiStatusResponse = new MultiStatusResponse() const id_type = hookListType ?? audienceSettings.external_id_type // Extract user identifiers and validPayloadIndicesBitmap from payloads - const { addUserIdentifiers, removeUserIdentifiers, validPayloadIndicesBitmap } = extractBatchUserIdentifiers( - payloads, - id_type, - multiStatusResponse, - syncMode, - features, - audienceMemberships - ) + const { + addUserIdentifiers, + removeUserIdentifiers, + validPayloadIndicesBitmap, + addPayloadIndices, + removePayloadIndices + } = extractBatchUserIdentifiers(payloads, id_type, multiStatusResponse, syncMode, features, audienceMemberships) // Create offline user data job payload const offlineUserJobPayload = createOfflineUserJobPayload(externalAudienceId, payloads[0], settings.customerId) // Step1 :- Create an Offline user data job @@ -1056,7 +1075,7 @@ export const processBatchPayload = async ( request, addUserIdentifiers, resourceName, - validPayloadIndicesBitmap, + addPayloadIndices, failedPayloadIndices, multiStatusResponse, features, @@ -1069,7 +1088,7 @@ export const processBatchPayload = async ( request, removeUserIdentifiers, resourceName, - validPayloadIndicesBitmap, + removePayloadIndices, failedPayloadIndices, multiStatusResponse, features,