-
Notifications
You must be signed in to change notification settings - Fork 305
[Actions Core] [LinkedIn Audiences] [Linkedin Conversions] Fix LinkedIn token propagation 401s with refresh and retry #3708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
e39d6fc
58502ec
e4fc510
82a51a5
1246404
391b2c3
0255433
0c47c61
eebb75d
a96cf93
4c5c523
7ea5b47
5635b9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { ErrorCodes, IntegrationError } from '../errors' | ||
| import { ErrorCodes, IntegrationError, RefreshTokenAndRetryError, RetryableError } from '../errors' | ||
| import { ActionDefinition, MultiStatusResponse } from '../destination-kit/action' | ||
| import { | ||
| StateContext, | ||
|
|
@@ -1452,6 +1452,55 @@ describe('destination kit', () => { | |
| ]) | ||
| expect(spy).toHaveBeenCalledTimes(0) | ||
| }) | ||
|
|
||
| test('should throw RetryableError without token refresh when RefreshTokenAndRetryError is thrown', async () => { | ||
| const destinationWithPropagationError: DestinationDefinition<JSONObject> = { | ||
| name: 'Test Propagation Error Destination', | ||
| mode: 'cloud', | ||
| authentication: authentication, | ||
| actions: { | ||
| customEvent: { | ||
| title: 'Send a Custom Event', | ||
| description: 'Send events to a custom event in API', | ||
| defaultSubscription: 'type = "track"', | ||
| fields: { | ||
| advertiserId: { | ||
| label: 'Advertiser ID', | ||
| description: 'Advertiser Id', | ||
| type: 'string', | ||
| required: true | ||
| } | ||
| }, | ||
| perform: () => { | ||
| throw new RefreshTokenAndRetryError('Token not yet propagated (serviceErrorCode 65601)') | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const destinationTest = new Destination(destinationWithPropagationError) | ||
| const testEvent: SegmentEvent = { | ||
| properties: { a: 'foo' }, | ||
| userId: '3456fff', | ||
| type: 'track' | ||
| } | ||
| const testSettings = { | ||
| apiSecret: 'test_key', | ||
| subscription: { | ||
| subscribe: 'type = "track"', | ||
| partnerAction: 'customEvent', | ||
| mapping: { | ||
| advertiserId: '1231241241' | ||
| } | ||
| }, | ||
| oauth: { | ||
| access_token: 'some-access-token', | ||
| refresh_token: 'refresh-token' | ||
| } | ||
| } | ||
| // handleError should throw RetryableError without calling refresh (token is already fresh) | ||
| await expect(destinationTest.onEvent(testEvent, testSettings)).rejects.toThrow(RetryableError) | ||
| }) | ||
|
Comment on lines
+1456
to
+1515
|
||
| }) | ||
| describe('onBatch', () => { | ||
| test('should refresh the access-token in case of Unauthorized(401)', async () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,14 @@ import type { | |
| ResultMultiStatusNode | ||
| } from './types' | ||
| import type { AllRequestOptions } from '../request-client' | ||
| import { ErrorCodes, IntegrationError, InvalidAuthenticationError, MultiStatusErrorReporter } from '../errors' | ||
| import { | ||
| ErrorCodes, | ||
| IntegrationError, | ||
| InvalidAuthenticationError, | ||
| MultiStatusErrorReporter, | ||
| RefreshTokenAndRetryError, | ||
| RetryableError | ||
| } from '../errors' | ||
| import { AuthTokens, getAuthData, getOAuth2Data, updateOAuthSettings } from './parse-settings' | ||
| import { InputData, Features } from '../mapping-kit' | ||
| import { retry } from '../retry' | ||
|
|
@@ -1022,6 +1029,12 @@ export class Destination<Settings = JSONObject, AudienceSettings = JSONObject> { | |
| settings: JSONObject, | ||
| options?: OnEventOptions | ||
| ): Promise<JSONObject> { | ||
| // Handle RefreshTokenAndRetryError: token was just refreshed but hasn't propagated yet. | ||
| // Just retry later — no need to refresh again. | ||
| if (error instanceof RefreshTokenAndRetryError) { | ||
| throw new RetryableError(error.message, 503) | ||
| } | ||
|
harsh-joshi99 marked this conversation as resolved.
Outdated
Comment on lines
+1032
to
+1043
|
||
|
|
||
| const statusCode = (error as ResponseError).status ?? (error as HTTPError)?.response?.status ?? 500 | ||
| const needsReauthentication = | ||
| statusCode === 401 && | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,16 @@ | ||
| import https from 'https' | ||
|
|
||
| import type { DestinationDefinition, ModifiedResponse } from '@segment/actions-core' | ||
| import { InvalidAuthenticationError, IntegrationError, ErrorCodes } from '@segment/actions-core' | ||
| import { | ||
| InvalidAuthenticationError, | ||
| IntegrationError, | ||
| ErrorCodes, | ||
| RefreshTokenAndRetryError | ||
| } from '@segment/actions-core' | ||
|
|
||
| import type { Settings } from './generated-types' | ||
| import updateAudience from './updateAudience' | ||
| import { LINKEDIN_API_VERSION } from './constants' | ||
| import { LINKEDIN_API_VERSION, LINKEDIN_TOKEN_PROPAGATION_ERROR_CODES } from './constants' | ||
| import { LinkedInAudiences } from './api' | ||
| import type { | ||
| RefreshTokenResponse, | ||
|
|
@@ -148,7 +153,21 @@ const destination: DestinationDefinition<Settings> = { | |
| authorization: `Bearer ${auth?.accessToken}`, | ||
| 'LinkedIn-Version': LINKEDIN_API_VERSION | ||
| }, | ||
| agent | ||
| agent, | ||
| afterResponse: [ | ||
| (_request: unknown, _options: unknown, response: { status: number; data: unknown }) => { | ||
| if (response.status === 401) { | ||
| const body = response.data as Record<string, unknown> | undefined | ||
| const serviceErrorCode = body?.serviceErrorCode as number | undefined | ||
| if (serviceErrorCode && LINKEDIN_TOKEN_PROPAGATION_ERROR_CODES.includes(serviceErrorCode)) { | ||
| throw new RefreshTokenAndRetryError( | ||
| `LinkedIn eventual consistency: token not yet propagated (serviceErrorCode ${serviceErrorCode})` | ||
| ) | ||
| } | ||
| } | ||
| return response | ||
| } | ||
| ] | ||
| } | ||
|
Comment on lines
+151
to
171
|
||
| }, | ||
|
Comment on lines
+146
to
172
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| import type { DestinationDefinition } from '@segment/actions-core' | ||
| import { InvalidAuthenticationError, IntegrationError, ErrorCodes } from '@segment/actions-core' | ||
| import { | ||
| InvalidAuthenticationError, | ||
| IntegrationError, | ||
| ErrorCodes, | ||
| RefreshTokenAndRetryError | ||
| } from '@segment/actions-core' | ||
| import type { Settings } from './generated-types' | ||
| import { LinkedInConversions } from './api' | ||
| import type { LinkedInTestAuthenticationError, RefreshTokenResponse, LinkedInRefreshTokenError } from './types' | ||
| import { LINKEDIN_API_VERSION } from './constants' | ||
| import { LINKEDIN_API_VERSION, LINKEDIN_TOKEN_PROPAGATION_ERROR_CODES } from './constants' | ||
| import https from 'https' | ||
| import streamConversion from './streamConversion' | ||
|
|
||
|
|
@@ -100,7 +105,21 @@ const destination: DestinationDefinition<Settings> = { | |
| 'LinkedIn-Version': LINKEDIN_API_VERSION, | ||
| 'X-Restli-Protocol-Version': `2.0.0` | ||
| }, | ||
| agent | ||
| agent, | ||
| afterResponse: [ | ||
| (_request: unknown, _options: unknown, response: { status: number; data: unknown }) => { | ||
| if (response.status === 401) { | ||
| const body = response.data as Record<string, unknown> | undefined | ||
| const serviceErrorCode = body?.serviceErrorCode as number | undefined | ||
| if (serviceErrorCode && LINKEDIN_TOKEN_PROPAGATION_ERROR_CODES.includes(serviceErrorCode)) { | ||
| throw new RefreshTokenAndRetryError( | ||
| `LinkedIn eventual consistency: token not yet propagated (serviceErrorCode ${serviceErrorCode})` | ||
| ) | ||
| } | ||
| } | ||
| return response | ||
| } | ||
| ] | ||
|
Comment on lines
+108
to
+122
|
||
| } | ||
|
Comment on lines
+96
to
123
|
||
| }, | ||
| actions: { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.