Skip to content

Commit

Permalink
[CCP-624] - Accept shouldRetryOnRetryableError in engage data feed ac…
Browse files Browse the repository at this point in the history
…tion config + other misc. updates (#1757)

* add changes

* move api lookups to utils

* pass features in local server

* add maxResponseSize test

* uncomment envs in sendEmail test

* fix multi tag push + typo

* reduce datadog stats cardinality

* add data feed failure reason to sendgrid performer datadog stat
  • Loading branch information
ryanrouleau authored Dec 12, 2023
1 parent 7492fbb commit 05c6199
Show file tree
Hide file tree
Showing 10 changed files with 244 additions and 83 deletions.
3 changes: 2 additions & 1 deletion packages/cli/src/lib/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,8 @@ function setupRoutes(def: DestinationDefinition | null): void {
settings: req.body.settings || {},
audienceSettings: req.body.payload?.context?.personas?.audience_settings || {},
mapping: mapping || req.body.payload || {},
auth: req.body.auth || {}
auth: req.body.auth || {},
features: req.body.features || {}
}

if (Array.isArray(eventParams.data)) {
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/destination-kit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,9 @@ export interface Logger {

export interface DataFeedCache {
setRequestResponse(requestId: string, response: string, expiryInSeconds: number): Promise<void>
getRequestResponse(requestId: string): Promise<string>
getRequestResponse(requestId: string): Promise<string | null>
maxResponseSizeBytes: number
maxExpirySeconds: number
}

export class Destination<Settings = JSONObject, AudienceSettings = JSONObject> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Sendgrid from '..'
import { FLAGON_NAME_LOG_ERROR, FLAGON_NAME_LOG_INFO, SendabilityStatus } from '../../utils'
import { loggerMock, expectErrorLogged, expectInfoLogged } from '../../utils/testUtils'
import { insertEmailPreviewText } from '../sendEmail/insertEmailPreviewText'
import { FLAGON_NAME_DATA_FEEDS } from '../previewApiLookup'
import { FLAGON_NAME_DATA_FEEDS } from '../../utils/apiLookups'

const sendgrid = createTestIntegration(Sendgrid)
const timestamp = new Date().toISOString()
Expand Down Expand Up @@ -1954,7 +1954,7 @@ describe.each([
})

describe('api lookups', () => {
it('are called and responses are passed to email body liquid renderer before sending', async () => {
it('are called and successful responses are passed to email body liquid renderer before sending', async () => {
const sendGridRequest = nock('https://api.sendgrid.com')
.post('/v3/mail/send', {
...sendgridRequestBody,
Expand Down Expand Up @@ -2022,16 +2022,17 @@ describe.each([
}
],
bodyHtml:
'Current temperature: {{lookups.weather.current.temperature}}, Current bitcoin price: {{lookups.btcPrice.current.price}}'
'Current temperature: {{datafeeds.weather.current.temperature}}, Current bitcoin price: {{datafeeds.btcPrice.current.price}}'
})
})

expect(responses.length).toBeGreaterThan(0)
expect(sendGridRequest.isDone()).toBe(true)
})

it('should throw error if at least one api lookup fails', async () => {
nock(`https://fakeweather.com`).get('/api').reply(429)
it('should rethrow request client error if at least one api lookup fails with shouldRetryOnRetryableError == true', async () => {
const dataFeedNock = nock(`https://fakeweather.com`).get('/api').reply(429)
const sendGridRequest = nock('https://api.sendgrid.com').post('/v3/mail/send', sendgridRequestBody).reply(200, {})

await expect(
sendgrid.testAction('sendEmail', {
Expand All @@ -2052,20 +2053,75 @@ describe.each([
}),
settings,
mapping: getDefaultMapping({
apiLookups: {
apiLookups: [
{
id: '1',
name: 'weather',
url: 'https://fakeweather.com/api',
method: 'get',
cacheTtl: 0,
responseType: 'json',
shouldRetryOnRetryableError: true
}
]
})
})
).rejects.toThrowError('Too Many Requests')

expect(dataFeedNock.isDone()).toEqual(true)
expect(sendGridRequest.isDone()).toEqual(false)
expectErrorLogged('Too Many Requests')
})

it('should send message with empty data if api lookup fails with shouldRetryOnRetryableError == false', async () => {
const sendGridRequest = nock('https://api.sendgrid.com')
.post('/v3/mail/send', {
...sendgridRequestBody,
content: [
{
type: 'text/html',
value: `<html><head></head><body>Current temperature: 99</body></html>`
}
]
})
.reply(200, {})
const dataFeedNock = nock(`https://fakeweather.com`).get('/api').reply(429)

await sendgrid.testAction('sendEmail', {
...defaultActionProps,
event: createMessagingTestEvent({
timestamp,
event: 'Audience Entered',
userId: userData.userId,
external_ids: [
{
collection: 'users',
encoding: 'none',
id: userData.email,
isSubscribed: true,
type: 'email'
}
]
}),
settings,
mapping: getDefaultMapping({
apiLookups: [
{
id: '1',
name: 'weather',
url: 'https://fakeweather.com/api',
method: 'get',
cacheTtl: 0,
responseType: 'json'
responseType: 'json',
shouldRetryOnRetryableError: false
}
})
],
bodyHtml: 'Current temperature: {{datafeeds.weather.current.temperature | default: 99 }}'
})
).rejects.toThrowError('Too Many Requests')
})

const sendGridRequest = nock('https://api.sendgrid.com').post('/v3/mail/send', sendgridRequestBody).reply(200, {})
expect(sendGridRequest.isDone()).toEqual(false)
expect(dataFeedNock.isDone()).toBe(true)
expect(sendGridRequest.isDone()).toEqual(true)
expectErrorLogged('Too Many Requests')
})
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { ActionDefinition } from '@segment/actions-core'
import { Settings } from '../generated-types'
import { Payload } from './generated-types'
import { apiLookupActionFields, performApiLookup } from './api-lookups'
import { Profile } from '../Profile'
import { apiLookupActionFields, performApiLookup } from '../../utils/apiLookups'
import { Profile } from '../../utils/Profile'

export const actionDefinition: ActionDefinition<Settings, Payload> = {
title: 'Perform a single API lookup',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from './actionDefinition'
export * from './api-lookups'
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { ExtId, MessageSendPerformer, OperationContext, ResponseError, track } from '../../utils'
import type { Settings } from '../generated-types'
import type { Payload } from './generated-types'
import { Profile } from '../Profile'
import { Profile } from '../../utils/Profile'
import { Liquid as LiquidJs } from 'liquidjs'
import { IntegrationError, RequestOptions } from '@segment/actions-core'
import { ApiLookupConfig, apiLookupLiquidKey, performApiLookup } from '../previewApiLookup'
import { ApiLookupConfig, FLAGON_NAME_DATA_FEEDS, apiLookupLiquidKey, performApiLookup } from '../../utils/apiLookups'
import { insertEmailPreviewText } from './insertEmailPreviewText'
import cheerio from 'cheerio'
import { isRestrictedDomain } from './isRestrictedDomain'
Expand Down Expand Up @@ -129,8 +129,14 @@ export class SendEmailPerformer extends MessageSendPerformer<Settings, Payload>
])

let apiLookupData = {}
if (this.isFeatureActive('is-datafeeds-enabled')) {
apiLookupData = await this.performApiLookups(this.payload.apiLookups, profile)
if (this.isFeatureActive(FLAGON_NAME_DATA_FEEDS)) {
try {
apiLookupData = await this.performApiLookups(this.payload.apiLookups, profile)
} catch (error) {
// Catching error to add tags, rethrowing to continue bubbling up
this.tags.push('reason:data_feed_failure')
throw error
}
}

const parsedBodyHtml = await this.getBodyHtml(profile, apiLookupData, emailProfile)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ActionDefinition } from '@segment/actions-core'
import type { Settings } from '../generated-types'
import type { Payload } from './generated-types'
import { apiLookupActionFields } from '../previewApiLookup'
import { apiLookupActionFields } from '../../utils/apiLookups'
import { SendEmailPerformer } from './SendEmailPerformer'

export const actionDefinition: ActionDefinition<Settings, Payload> = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import nock from 'nock'
import { ApiLookupConfig, getRequestId, performApiLookup } from '../previewApiLookup'
import { ApiLookupConfig, getRequestId, performApiLookup } from '../apiLookups'
import { DataFeedCache } from '../../../../../../core/src/destination-kit/index'
import createRequestClient from '../../../../../../core/src/create-request-client'
import { DataFeedCache } from '../../../../../../core/src/destination-kit/'

const profile = {
traits: {
Expand Down Expand Up @@ -37,15 +37,18 @@ const cachedApiLookup = {
cacheTtl: 60000
}

const createMockRequestStore = () => {
const createMockRequestStore = (overrides?: Partial<DataFeedCache>) => {
const mockStore: Record<string, any> = {}
const mockDataFeedCache: DataFeedCache = {
setRequestResponse: jest.fn(async (requestId, response) => {
mockStore[requestId] = response
}),
getRequestResponse: jest.fn(async (requestId) => {
return mockStore[requestId]
})
}),
maxExpirySeconds: 600000,
maxResponseSizeBytes: 1000000,
...overrides
}
return mockDataFeedCache
}
Expand Down Expand Up @@ -91,7 +94,83 @@ describe('api lookups', () => {
})
})

it('rethrows error when shouldRetryOnRetryableError is true and api call fails', async () => {
const apiLookupRequest = nock(`https://fakeweather.com`).get(`/api/current`).reply(429)

await expect(
performApiLookup(
request,
{
...nonCachedApiLookup,
shouldRetryOnRetryableError: true
},
profile,
undefined,
[],
settings,
undefined,
undefined
)
).rejects.toThrowError()

expect(apiLookupRequest.isDone()).toEqual(true)
})

it('does not rethrow error and returns empty object when shouldRetryOnRetryableError is false and api call fails', async () => {
const apiLookupRequest = nock(`https://fakeweather.com`).get(`/api/current`).reply(429)

const data = await performApiLookup(
request,
{
...nonCachedApiLookup,
shouldRetryOnRetryableError: false
},
profile,
undefined,
[],
settings,
undefined,
undefined
)

expect(apiLookupRequest.isDone()).toEqual(true)
expect(data).toEqual({})
})

describe('when cacheTtl > 0', () => {
it('throws error if cache is not available', async () => {
const apiLookupRequest = nock(`https://fakeweather.com`)
.get(`/api/current`)
.reply(200, {
current: {
temperature: 70
}
})

await expect(
performApiLookup(request, cachedApiLookup, profile, undefined, [], settings, undefined, undefined)
).rejects.toThrowError('Data feed cache not available and cache needed')

expect(apiLookupRequest.isDone()).toEqual(false)
})

it('throws error if response size is too big', async () => {
const mockDataFeedCache = createMockRequestStore({ maxResponseSizeBytes: 1 })
const apiLookupRequest = nock(`https://fakeweather.com`)
.get(`/api/current`)
.reply(200, {
current: {
temperature: 70
}
})

await expect(
performApiLookup(request, cachedApiLookup, profile, undefined, [], settings, undefined, mockDataFeedCache)
).rejects.toThrowError('Data feed response size too big too cache and caching needed, failing send')

expect(apiLookupRequest.isDone()).toEqual(true)
})

it('sets cache when cache miss', async () => {
const mockDataFeedCache = createMockRequestStore()
const apiLookupRequest = nock(`https://fakeweather.com`)
Expand Down Expand Up @@ -159,7 +238,7 @@ describe('api lookups', () => {
})
})

describe('cached responses are unique when rendered', () => {
describe('cached responses are unique dependent on api config post liquid rendering value', () => {
const profiles = [{ traits: { lastName: 'Browning' } }, { traits: { lastName: 'Smith' } }]

it('url is different', async () => {
Expand Down
Loading

0 comments on commit 05c6199

Please sign in to comment.