Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,151 @@ describe('MS Bing Ads Audiences syncAudiences', () => {
expect(payloadArg[0].email).toBe('add1@segment.com')
})

describe('debug logging (actions-ms-bing-ads-audiences-debug-logging flag)', () => {
const DEBUG_FLAG = 'actions-ms-bing-ads-audiences-debug-logging'

const makeLogger = () => ({ info: jest.fn(), warn: jest.fn(), error: jest.fn() } as any)

const addEvent = () =>
createTestEvent({
type: 'identify',
properties: { aud_key: true },
context: { traits: { email: 'demo@segment.com' } }
})

it('does not log when the flag is off', async () => {
nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, {})
const logger = makeLogger()

await testDestination.testAction('syncAudiences', {
event: addEvent(),
mapping: baseMapping,
useDefaultMappings: true,
settings,
logger,
features: { [DEBUG_FLAG]: false }
})

expect(logger.warn).not.toHaveBeenCalled()
})

it('logs the tracking id and metadata when on, without leaking hashed identifiers', async () => {
nock(BASE_URL)
.post('/CustomerListUserData/Apply')
.reply(200, { PartialErrors: [] }, { TrackingId: 'abc-123-track' })
const logger = makeLogger()

await testDestination.testAction('syncAudiences', {
event: addEvent(),
mapping: baseMapping,
useDefaultMappings: true,
settings,
logger,
features: { [DEBUG_FLAG]: true }
})

expect(logger.warn).toHaveBeenCalledTimes(1)
const logged = (logger.warn as jest.Mock).mock.calls[0][0] as string
expect(logged).toContain('[ms-bing-ads-audiences][DEBUG]')
expect(logged).toContain('trackingId=abc-123-track')
expect(logged).toContain('identifierType=Email')
expect(logged).toContain('itemCount=1')
// The hashed identifier must never be logged.
expect(logged).not.toContain('5a95f052958dac8ed1d66d74eb481b3ccdbbc953b583c5ff0325be6b091d6281')
})

it('falls back to a body-level tracking id when no header is present', async () => {
nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { TrackingId: 'body-track-789', PartialErrors: [] })
const logger = makeLogger()

await testDestination.testAction('syncAudiences', {
event: addEvent(),
mapping: baseMapping,
useDefaultMappings: true,
settings,
logger,
features: { [DEBUG_FLAG]: true }
})

const logged = (logger.warn as jest.Mock).mock.calls[0][0] as string
expect(logged).toContain('trackingId=body-track-789')
})

it('redacts PartialError free-text fields that can echo identifiers', async () => {
nock(BASE_URL)
.post('/CustomerListUserData/Apply')
.reply(200, {
PartialErrors: [
{
ErrorCode: 'InvalidCustomerListItem',
Code: 4001,
Index: 0,
Type: 'BatchError',
Message: 'Invalid value crm_secret_12345',
Details: 'crm_secret_12345',
FieldPath: 'CustomerListItems[0]=crm_secret_12345'
}
]
})
const logger = makeLogger()

await testDestination.testBatchAction('syncAudiences', {
events: [addEvent()],
mapping: baseMapping,
useDefaultMappings: true,
settings,
logger,
features: { [DEBUG_FLAG]: true }
})

const logged = (logger.warn as jest.Mock).mock.calls[0][0] as string
expect(logged).toContain('InvalidCustomerListItem')
// The free-text fields (and any identifier they echo) must not be logged.
expect(logged).not.toContain('crm_secret_12345')
expect(logged).not.toContain('Message')
expect(logged).not.toContain('FieldPath')
})

it('does not let a throwing logger break the delivery', async () => {
// Debug logging runs after Bing has accepted the records; a throwing logger must not
// fail the delivery (which would trigger a duplicate re-send on retry).
nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { PartialErrors: [] })
const logger = makeLogger()
;(logger.warn as jest.Mock).mockImplementation(() => {
throw new Error('logger down')
})

const response = await testDestination.testAction('syncAudiences', {
event: addEvent(),
mapping: baseMapping,
useDefaultMappings: true,
settings,
logger,
features: { [DEBUG_FLAG]: true }
})

expect(response[0].status).toBe(200)
})

it('does not log on the error path (only success responses are logged)', async () => {
nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' })
const logger = makeLogger()

const response = await testDestination.testBatchAction('syncAudiences', {
events: [addEvent()],
mapping: baseMapping,
useDefaultMappings: true,
settings,
logger,
features: { [DEBUG_FLAG]: true }
})

expect(logger.warn).not.toHaveBeenCalled()
expect(utils.handleHttpError).toHaveBeenCalled()
expect(response[0].status).toBe(500)
})
})

it('should throw non-HTTP errors in batch mode', async () => {
// Create a custom error that is NOT an HTTPError
const customError = new Error('Custom non-HTTP error')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ActionDefinition, RequestClient, HTTPError } from '@segment/actions-core'
import { ActionDefinition, RequestClient, HTTPError, Logger, ModifiedResponse } from '@segment/actions-core'
import { MultiStatusResponse } from '@segment/actions-core'
import type { Settings } from '../generated-types'
import type { Payload } from './generated-types'
Expand All @@ -20,7 +20,7 @@ import {
handleHttpError,
categorizePayloadByAction
} from '../utils'
import { Identifier, SyncAudiencePayload } from '../types'
import { Identifier, SyncAudiencePayload, PartialError } from '../types'

const action: ActionDefinition<Settings, Payload> = {
title: 'Sync Audiences',
Expand All @@ -36,12 +36,65 @@ const action: ActionDefinition<Settings, Payload> = {
batch_size,
computation_class
},
perform: async (request, { payload }) => {
return await syncUser(request, [payload], false)
perform: async (request, { payload, features, logger }) => {
return await syncUser(request, [payload], false, isDebugLoggingEnabled(features), logger)
},

performBatch: async (request, { payload }) => {
return await syncUser(request, payload, true)
performBatch: async (request, { payload, features, logger }) => {
return await syncUser(request, payload, true, isDebugLoggingEnabled(features), logger)
}
}

// TEMPORARY DEBUG LOGGING — gated behind the 'actions-ms-bing-ads-audiences-debug-logging' feature
// flag (off by default). Logs Microsoft's tracking id (needed to file Bing Ads support tickets)
// plus non-sensitive request metadata and a redacted error summary. It intentionally does NOT log
// CustomerListItems (hashed emails / unhashed CRM ids) or the PartialError free-text fields
// (Message/Details/FieldPath), which can echo back an identifier. Remove once debugging is done.
const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging'

const isDebugLoggingEnabled = (features: Record<string, boolean> | undefined): boolean =>
Boolean(features?.[DEBUG_LOGGING_FLAG])

// Microsoft returns its request/tracking id as a response header (and sometimes echoes it in the
// body). It's a short opaque id, not PII.
const extractTrackingId = (response: ModifiedResponse): string => {
const header = response.headers?.get('trackingid') || response.headers?.get('x-ms-trackingid')
if (header) return header
const body = response.data as { TrackingId?: string } | undefined
return body?.TrackingId || 'none'
}
Comment on lines +60 to +65

// Reduce PartialErrors to codes + index; the free-text fields can echo back the offending
// identifier so they are dropped.
const summarizeErrors = (errors: PartialError[] | undefined): string =>
JSON.stringify((errors ?? []).map((e) => ({ ErrorCode: e.ErrorCode, Code: e.Code, Index: e.Index, Type: e.Type })))

// TEMPORARY: log a redacted, PII-safe summary of a successful Bing Ads response.
const logBingAdsResponse = (
logger: Logger | undefined,
debugLogging: boolean,
action: string,
audienceId: string,
sentPayload: SyncAudiencePayload,
response: ModifiedResponse
): void => {
Comment thread
harsh-joshi99 marked this conversation as resolved.
Comment on lines +73 to +80
Comment on lines +73 to +80
if (!debugLogging || !logger) return
// Isolate from delivery control flow: this runs inside the syncUser try/catch after Bing has
// already accepted the records, so a throwing/partial logger must never fail an otherwise
// successful (batch) delivery or trigger a duplicate re-send on retry.
try {
const { CustomerListItemSubType, CustomerListItems } = sentPayload.CustomerListUserData
const partialErrors = (response.data as { PartialErrors?: PartialError[] } | undefined)?.PartialErrors
const line =
`[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` +
`trackingId=${extractTrackingId(response)} identifierType=${CustomerListItemSubType} ` +
`itemCount=${CustomerListItems.length} partialErrors=${summarizeErrors(partialErrors)}`
Comment on lines +86 to +91
// Emit at warn: the delivery runtime's logger filters out info-level lines, so info never
// reaches the log pipeline. warn is the lowest level that reliably ships. Uses the optional
// ?.warn?.() call idiom so a partial logger no-ops rather than throwing.
logger?.warn?.(line.slice(0, 4096))
} catch {
// Best-effort debug logging — intentionally swallowed.
}
}

Expand All @@ -58,7 +111,13 @@ const action: ActionDefinition<Settings, Payload> = {
* @returns A promise that resolves to a `MultiStatusResponse` object summarizing the results.
* @throws Will throw an error if a non-batch operation fails, or rethrows non-HTTP errors in batch mode.
*/
const syncUser = async (request: RequestClient, payload: Payload[], isBatch: boolean) => {
const syncUser = async (
request: RequestClient,
payload: Payload[],
isBatch: boolean,
debugLogging = false,
logger?: Logger
) => {
const msResponse = new MultiStatusResponse()

if (!Array.isArray(payload) || payload.length === 0) {
Expand Down Expand Up @@ -87,10 +146,12 @@ const syncUser = async (request: RequestClient, payload: Payload[], isBatch: boo
// Send data to Microsoft Bing Ads for both Add and Remove actions if they have entries
if (addMap.size > 0) {
const response = await sendDataToMicrosoftBingAds(request, addPayload)
logBingAdsResponse(logger, debugLogging, 'Add', audienceId, addPayload, response)
handleMultistatusResponse(msResponse, response, addItems, addMap, payload, isBatch)
}
if (removeMap.size > 0) {
const response = await sendDataToMicrosoftBingAds(request, removePayload)
logBingAdsResponse(logger, debugLogging, 'Remove', audienceId, removePayload, response)
handleMultistatusResponse(msResponse, response, removeItems, removeMap, payload, isBatch)
}
} catch (error) {
Expand Down
Loading