-
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 all 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 |
|---|---|---|
|
|
@@ -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, | ||
| TokenPropagationRetryError, | ||
| RetryableError | ||
| } from '../errors' | ||
| import { AuthTokens, getAuthData, getOAuth2Data, updateOAuthSettings } from './parse-settings' | ||
| import { InputData, Features } from '../mapping-kit' | ||
| import { retry } from '../retry' | ||
|
|
@@ -1022,6 +1029,19 @@ export class Destination<Settings = JSONObject, AudienceSettings = JSONObject> { | |
| settings: JSONObject, | ||
| options?: OnEventOptions | ||
| ): Promise<JSONObject> { | ||
| // Handle TokenPropagationRetryError: LinkedIn returns serviceErrorCode 65601 for both | ||
| // token propagation delays and revoked/expired tokens. Refresh the token first so the | ||
| // retry always uses a fresh token. If the refresh itself fails (e.g. the refresh token | ||
| // is also expired), that error propagates instead of retrying forever with a dead token. | ||
| // RetryableError(503) signals Segment infrastructure to retry after backoff. | ||
| if (error instanceof TokenPropagationRetryError) { | ||
| const isOAuth = this.authentication?.scheme === 'oauth2' || this.authentication?.scheme === 'oauth-managed' | ||
| if (isOAuth) { | ||
| await this.handleAuthError(settings, options) | ||
| } | ||
| throw new RetryableError(error.message, 503) | ||
| } | ||
|
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -98,6 +98,33 @@ export class APIError extends IntegrationError { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Error indicating a 401 caused by OAuth token propagation delay (eventual | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * consistency) rather than true revocation. The token was recently refreshed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * and is valid but has not yet propagated across all provider nodes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The framework converts this into a RetryableError(503) so Segment | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * infrastructure retries the event after a backoff delay, by which time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the token will have propagated. No additional token refresh is performed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * on the exception-throwing path — handleError intercepts this error before | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the OAuth re-authentication logic runs. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * This error is currently thrown by LinkedIn destination hooks when | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * serviceErrorCode 65601 or 65602 appears in a 401 response body. Reuse for | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * other providers should only occur after confirming the same | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * retry-without-refresh semantic applies. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class TokenPropagationRetryError extends CustomError { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use 503 to match the RetryableError this converts into — do NOT use 401, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // which would cause generic status-based 401 handlers to trigger a token refresh. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+108
to
+119
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the token will have propagated. No additional token refresh is performed | |
| * on the exception-throwing path — handleError intercepts this error before | |
| * the OAuth re-authentication logic runs. | |
| * | |
| * This error is currently thrown by LinkedIn destination hooks when | |
| * serviceErrorCode 65601 or 65602 appears in a 401 response body. Reuse for | |
| * other providers should only occur after confirming the same | |
| * retry-without-refresh semantic applies. | |
| */ | |
| export class TokenPropagationRetryError extends CustomError { | |
| // Use 503 to match the RetryableError this converts into — do NOT use 401, | |
| // which would cause generic status-based 401 handlers to trigger a token refresh. | |
| * the token will typically have propagated. This error is intended to model | |
| * a transient, retryable propagation delay rather than a permanent invalid- | |
| * credentials condition. Framework auth-error handling may still run as part | |
| * of the normal exception path, so callers should not rely on this error type | |
| * to suppress OAuth refresh logic. | |
| * | |
| * This error is currently thrown by LinkedIn destination hooks when | |
| * serviceErrorCode 65601 or 65602 appears in a 401 response body. Reuse for | |
| * other providers should only occur after confirming the same | |
| * transient retry semantic applies. | |
| */ | |
| export class TokenPropagationRetryError extends CustomError { | |
| // Use 503 to match the RetryableError this converts into and to represent | |
| // a transient retryable condition instead of a standard invalid-auth 401. |
| 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, | ||
| TokenPropagationRetryError | ||
| } 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 TokenPropagationRetryError( | ||
| `LinkedIn eventual consistency: token not yet propagated (serviceErrorCode ${serviceErrorCode})` | ||
| ) | ||
| } | ||
| } | ||
| return response | ||
| } | ||
| ] | ||
|
Comment on lines
+108
to
+122
|
||
| } | ||
| }, | ||
| actions: { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new test asserts
refreshAccessTokenis called whenTokenPropagationRetryErroris thrown, which conflicts with the PR description's test plan claim thatrefreshAccessTokenis NOT called. Please reconcile the expected behavior: either adjust the test and implementation to avoid refresh on token-propagation retries, or update the PR description/test plan to reflect that a refresh is performed.