Skip to content
Merged
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
53 changes: 52 additions & 1 deletion src/platform/auth/unified/remintRetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@ import {
fetchWithUnifiedRemint
} from '@/platform/auth/unified/remintRetry'

const { mockRemint, flagState } = vi.hoisted(() => ({
const { mockRemint, mockTrackUnifiedAuthRetry, flagState } = vi.hoisted(() => ({
mockRemint: vi.fn(),
mockTrackUnifiedAuthRetry: vi.fn(),
flagState: { unifiedCloudAuthEnabled: true }
}))

vi.mock('@/platform/telemetry', () => ({
useTelemetry: () => ({
trackUnifiedAuthRetry: mockTrackUnifiedAuthRetry
})
}))
Comment thread
coderabbitai[bot] marked this conversation as resolved.

vi.mock('@/platform/workspace/stores/workspaceAuthStore', () => ({
useWorkspaceAuthStore: () => ({ remintUnifiedOnce: mockRemint })
}))
Expand All @@ -37,6 +44,7 @@ describe('fetchWithUnifiedRemint', () => {

beforeEach(() => {
mockRemint.mockReset()
mockTrackUnifiedAuthRetry.mockReset()
flagState.unifiedCloudAuthEnabled = true
mockFetch = vi.fn()
vi.stubGlobal('fetch', mockFetch)
Expand Down Expand Up @@ -64,6 +72,11 @@ describe('fetchWithUnifiedRemint', () => {
const retryHeaders = new Headers(mockFetch.mock.calls[1][1].headers)
expect(retryHeaders.get('Authorization')).toBe('Bearer tokenB')
expect(retryHeaders.get('Comfy-User')).toBe('u1')
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'fetch',
outcome: 'succeeded',
final_status: 200
})
})

it('surfaces a persistent 401 after exactly one retry (AC2)', async () => {
Expand All @@ -82,6 +95,12 @@ describe('fetchWithUnifiedRemint', () => {
expect(result).toBe(secondUnauthorized)
expect(mockFetch).toHaveBeenCalledTimes(2)
expect(mockRemint).toHaveBeenCalledTimes(1)
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'fetch',
outcome: 'failed',
final_status: 401,
failure_reason: 'retry_rejected'
})
})

it('does not re-mint or retry when the caller gate is false (AC3)', async () => {
Expand All @@ -96,6 +115,7 @@ describe('fetchWithUnifiedRemint', () => {
expect(result).toBe(unauthorized)
expect(mockFetch).toHaveBeenCalledTimes(1)
expect(mockRemint).not.toHaveBeenCalled()
expect(mockTrackUnifiedAuthRetry).not.toHaveBeenCalled()
})

it('does not retry a non-401 response', async () => {
Expand Down Expand Up @@ -126,6 +146,12 @@ describe('fetchWithUnifiedRemint', () => {
expect(result).toBe(unauthorized)
expect(mockFetch).toHaveBeenCalledTimes(1)
expect(mockRemint).toHaveBeenCalledTimes(1)
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'fetch',
outcome: 'failed',
final_status: 401,
failure_reason: 'remint_failed'
})
})

it('uses the bearer from a Request when init does not override headers', async () => {
Expand Down Expand Up @@ -178,6 +204,12 @@ describe('fetchWithUnifiedRemint', () => {
expect(result).toBe(unauthorized)
expect(mockFetch).toHaveBeenCalledTimes(1)
expect(mockRemint).not.toHaveBeenCalled()
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'fetch',
outcome: 'failed',
final_status: 401,
failure_reason: 'missing_bearer'
})
})

it('surfaces the original 401 when the re-mint throws a permanent auth error', async () => {
Expand Down Expand Up @@ -212,6 +244,12 @@ describe('fetchWithUnifiedRemint', () => {
expect(result).toBe(unauthorized)
expect(mockFetch).toHaveBeenCalledTimes(1)
expect(mockRemint).not.toHaveBeenCalled()
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'fetch',
outcome: 'failed',
final_status: 401,
failure_reason: 'non_replayable_body'
})
})

it.for([
Expand Down Expand Up @@ -259,6 +297,7 @@ describe('fetchWithUnifiedRemint', () => {
describe('attachUnifiedRemintInterceptor', () => {
beforeEach(() => {
mockRemint.mockReset()
mockTrackUnifiedAuthRetry.mockReset()
flagState.unifiedCloudAuthEnabled = true
})

Expand Down Expand Up @@ -312,6 +351,11 @@ describe('attachUnifiedRemintInterceptor', () => {
expect(String(adapter.mock.calls[1][0].headers.Authorization)).toBe(
'Bearer tokenB'
)
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'axios',
outcome: 'succeeded',
final_status: 200
})
})

it('retries once then surfaces a persistent 401 (AC2)', async () => {
Expand All @@ -326,6 +370,12 @@ describe('attachUnifiedRemintInterceptor', () => {

expect(adapter).toHaveBeenCalledTimes(2)
expect(mockRemint).toHaveBeenCalledTimes(1)
expect(mockTrackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'axios',
outcome: 'failed',
final_status: 401,
failure_reason: 'retry_rejected'
})
})

it('does not re-mint when the flag is OFF (AC3)', async () => {
Expand All @@ -340,6 +390,7 @@ describe('attachUnifiedRemintInterceptor', () => {

expect(adapter).toHaveBeenCalledTimes(1)
expect(mockRemint).not.toHaveBeenCalled()
expect(mockTrackUnifiedAuthRetry).not.toHaveBeenCalled()
})

it('does not re-mint a request flagged __skipUnifiedRemint (acceptInvite)', async () => {
Expand Down
70 changes: 67 additions & 3 deletions src/platform/auth/unified/remintRetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import type {
import axios, { AxiosHeaders } from 'axios'

import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import type {
UnifiedAuthRetryFailureReason,
UnifiedAuthRetryMetadata
} from '@/platform/telemetry/types'

let cachedUnifiedFlags:
| { readonly unifiedCloudAuthEnabled: boolean }
Expand Down Expand Up @@ -47,6 +52,32 @@ async function tryRemintToken(expectedToken: string): Promise<string | null> {
}
}

function trackRetry(
transport: UnifiedAuthRetryMetadata['transport'],
outcome: UnifiedAuthRetryMetadata['outcome'],
finalStatus?: number,
failureReason?: UnifiedAuthRetryFailureReason
): void {
useTelemetry()?.trackUnifiedAuthRetry({
transport,
outcome,
...(finalStatus !== undefined && { final_status: finalStatus }),
...(failureReason !== undefined && { failure_reason: failureReason })
})
}

function trackRetryResponse(
transport: UnifiedAuthRetryMetadata['transport'],
status: number
): void {
trackRetry(
transport,
status < 400 ? 'succeeded' : 'failed',
status,
status < 400 ? undefined : 'retry_rejected'
)
}

function bearerToken(authorization: unknown): string | undefined {
if (typeof authorization !== 'string') return
return authorization.startsWith('Bearer ')
Expand Down Expand Up @@ -92,21 +123,33 @@ export async function fetchWithUnifiedRemint(
console.warn(
'fetchWithUnifiedRemint: a ReadableStream body is not replayable; surfacing the original 401'
)
trackRetry('fetch', 'failed', response.status, 'non_replayable_body')
return response
}

const requestHeaders = fetchRequestHeaders(input, init)
const expectedToken = bearerToken(requestHeaders.get('Authorization'))
if (!expectedToken) return response
if (!expectedToken) {
trackRetry('fetch', 'failed', response.status, 'missing_bearer')
return response
}

const token = await tryRemintToken(expectedToken)
if (!token) {
trackRetry('fetch', 'failed', response.status, 'remint_failed')
return response
}

const headers = requestHeaders
headers.set('Authorization', `Bearer ${token}`)
return fetch(retryInput, { ...init, headers })
try {
const retryResponse = await fetch(retryInput, { ...init, headers })
trackRetryResponse('fetch', retryResponse.status)
return retryResponse
} catch (error) {
trackRetry('fetch', 'failed', undefined, 'retry_request_failed')
throw error
}
}

function isRetriableUnauthorized(
Expand Down Expand Up @@ -142,11 +185,13 @@ export function attachUnifiedRemintInterceptor(client: AxiosInstance): void {
new AxiosHeaders(error.config.headers).get('Authorization')
)
if (!expectedToken) {
trackRetry('axios', 'failed', 401, 'missing_bearer')
throw error
}

const token = await tryRemintToken(expectedToken)
if (!token) {
trackRetry('axios', 'failed', 401, 'remint_failed')
throw error
}

Expand All @@ -155,7 +200,26 @@ export function attachUnifiedRemintInterceptor(client: AxiosInstance): void {
const { config } = error
const headers = new AxiosHeaders(config.headers)
headers.set('Authorization', `Bearer ${token}`)
return client.request({ ...config, headers, __unifiedRetried: true })
try {
const retryResponse = await client.request({
...config,
headers,
__unifiedRetried: true
})
trackRetryResponse('axios', retryResponse.status)
return retryResponse
} catch (retryError) {
const finalStatus = axios.isAxiosError(retryError)
? retryError.response?.status
: undefined
trackRetry(
'axios',
'failed',
finalStatus,
finalStatus !== undefined ? 'retry_rejected' : 'retry_request_failed'
)
throw retryError
}
}
)
}
21 changes: 21 additions & 0 deletions src/platform/telemetry/TelemetryRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,27 @@ describe('TelemetryRegistry', () => {
expect(b.trackAuthFailed).toHaveBeenCalledExactlyOnceWith(payload)
})

it('dispatches unified auth retry outcomes to supporting providers', () => {
const trackUnifiedAuthRetry = vi.fn()
const registry = new TelemetryRegistry()
registry.registerProvider({ trackUnifiedAuthRetry })
registry.registerProvider({})

registry.trackUnifiedAuthRetry({
transport: 'fetch',
outcome: 'failed',
final_status: 401,
failure_reason: 'retry_rejected'
})

expect(trackUnifiedAuthRetry).toHaveBeenCalledExactlyOnceWith({
transport: 'fetch',
outcome: 'failed',
final_status: 401,
failure_reason: 'retry_rejected'
})
})

it('dispatches trackAddApiCreditButtonClicked with its source', () => {
const provider: TelemetryProvider = {
trackAddApiCreditButtonClicked: vi.fn()
Expand Down
5 changes: 5 additions & 0 deletions src/platform/telemetry/TelemetryRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import type {
TemplateLibraryMetadata,
TemplateMetadata,
UiButtonClickMetadata,
UnifiedAuthRetryMetadata,
WidgetFavoriteToggledMetadata,
WorkflowCreatedMetadata,
WorkflowImportMetadata,
Expand Down Expand Up @@ -86,6 +87,10 @@ export class TelemetryRegistry implements TelemetryDispatcher {
this.dispatch((provider) => provider.trackAuthFailed?.(metadata))
}

trackUnifiedAuthRetry(metadata: UnifiedAuthRetryMetadata): void {
this.dispatch((provider) => provider.trackUnifiedAuthRetry?.(metadata))
}

trackUserLoggedIn(): void {
this.dispatch((provider) => provider.trackUserLoggedIn?.())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@ afterEach(() => {
})

describe('DatadogRumTelemetryProvider', () => {
it('records terminal unified auth retry outcomes without request data', () => {
new DatadogRumTelemetryProvider().trackUnifiedAuthRetry({
transport: 'axios',
outcome: 'failed',
final_status: 401,
failure_reason: 'retry_rejected'
})

expect(addAction).toHaveBeenCalledExactlyOnceWith(
TelemetryEvents.UNIFIED_AUTH_RETRY_FAILED,
{
transport: 'axios',
outcome: 'failed',
final_status: 401,
failure_reason: 'retry_rejected'
}
)
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it('records the same canonical billing name and context as PostHog', () => {
const event: BillingTelemetryEvent = {
operation: 'operation',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,25 @@ import { datadogRum } from '@datadog/browser-rum'
import type {
BillingTelemetryEvent,
ExecutionOutcomeMetadata,
TelemetryProvider
TelemetryProvider,
UnifiedAuthRetryMetadata
} from '../../types'
import {
getBillingTelemetryEventName,
getBillingTelemetryEventPayload
getBillingTelemetryEventPayload,
TelemetryEvents
} from '../../types'

export class DatadogRumTelemetryProvider implements TelemetryProvider {
trackUnifiedAuthRetry(metadata: UnifiedAuthRetryMetadata): void {
datadogRum.addAction(
metadata.outcome === 'succeeded'
? TelemetryEvents.UNIFIED_AUTH_RETRY_SUCCEEDED
: TelemetryEvents.UNIFIED_AUTH_RETRY_FAILED,
metadata
)
}

trackBillingEvent(event: BillingTelemetryEvent): void {
datadogRum.addAction(
getBillingTelemetryEventName(event),
Expand Down
Loading
Loading