diff --git a/packages/agent-bff/src/ai/ai-routes-middleware.ts b/packages/agent-bff/src/ai/ai-routes-middleware.ts index 9eadfdc198..eba4ad4e24 100644 --- a/packages/agent-bff/src/ai/ai-routes-middleware.ts +++ b/packages/agent-bff/src/ai/ai-routes-middleware.ts @@ -1,6 +1,7 @@ import type AiProxyClient from './ai-proxy-client'; import type { AiProxyResponse } from './ai-proxy-client'; import type { BffAccessTokenPayload } from '../oauth/bff-token'; +import type { EnvironmentIdResolver } from '../oauth/environment-id'; import type ForestServerClient from '../oauth/forest-server-client'; import type { SessionStore } from '../oauth/session-store'; import type { Logger } from '../ports/logger-port'; @@ -10,11 +11,13 @@ import { AiProxyTimeoutError } from './ai-proxy-client'; import { requireRenderingId } from '../auth/auth-mode'; import { unauthorized } from '../http/bff-http-error'; import { + environmentUnresolved, oauthRequired, upstreamError, upstreamTimeout, upstreamUnreachable, } from '../http/bff-local-errors'; +import { environmentFailureLevel } from '../oauth/environment-fetch-error'; import { OAuthRequestError } from '../oauth/oauth-error'; import ensureFreshServerAccess from '../oauth/session-lifecycle'; @@ -28,7 +31,7 @@ export interface AiRoutesMiddlewareOptions { client: AiProxyClient; sessionStore: SessionStore; serverClient: ForestServerClient; - environmentId?: number; + resolveEnvironmentId?: EnvironmentIdResolver; logger: Logger; } @@ -55,6 +58,32 @@ function describeFailure(error: unknown, depthLeft = MAX_CAUSE_DEPTH): string { return `${error.name}: ${clamp(error.message)}${tail}`; } +/** + * Resolved outside the `try` that maps AI proxy failures: a Forest server refusing the environment + * read is not the AI proxy being unreachable, and telling the operator otherwise sends them + * debugging a component that was never contacted. + */ +async function resolveEnvironmentIdOrRefuse( + resolveEnvironmentId: EnvironmentIdResolver | undefined, + logger: Logger, +): Promise { + if (!resolveEnvironmentId) return undefined; + + try { + return await resolveEnvironmentId(); + } catch (error) { + logger( + environmentFailureLevel(error), + 'AI query refused: the Forest environment id could not be resolved', + { cause: describeFailure(error) }, + ); + + if (error instanceof Error && error.name === 'TimeoutError') throw upstreamTimeout(); + + throw environmentUnresolved(); + } +} + async function resolveSessionAccessToken( principal: BffAccessTokenPayload, store: SessionStore, @@ -104,7 +133,7 @@ export default function createAiRoutesMiddleware({ client, sessionStore, serverClient, - environmentId, + resolveEnvironmentId, logger, }: AiRoutesMiddlewareOptions): Middleware { return async function aiRoutesMiddleware(ctx, next) { @@ -124,6 +153,8 @@ export default function createAiRoutesMiddleware({ logger, ); + const environmentId = await resolveEnvironmentIdOrRefuse(resolveEnvironmentId, logger); + let response: AiProxyResponse; try { diff --git a/packages/agent-bff/src/build-bff.ts b/packages/agent-bff/src/build-bff.ts index dc8ddd17af..9c07a49f77 100644 --- a/packages/agent-bff/src/build-bff.ts +++ b/packages/agent-bff/src/build-bff.ts @@ -1,4 +1,5 @@ import type { BFFConfig } from './config/env-config'; +import type { EnvironmentIdResolver } from './oauth/environment-id'; import type { SessionStore } from './oauth/session-store'; import type { UnfoldSource } from './openapi/unfolded-document'; import type { Logger } from './ports/logger-port'; @@ -30,6 +31,7 @@ import BODY_LIMIT, { AI_BODY_LIMIT } from './http/body-limit'; import createErrorMiddleware from './http/error-middleware'; import createHealthRoute from './http/health-route'; import createVersionHeaderMiddleware from './http/version-header-middleware'; +import createEnvironmentIdResolver, { tolerateEnvironmentIdFailure } from './oauth/environment-id'; import ForestServerClient from './oauth/forest-server-client'; import createOAuthRoutes from './oauth/oauth-routes'; import createInMemorySessionStore from './oauth/session-store'; @@ -134,11 +136,11 @@ interface OAuthSession { interface OAuthEdge { middlewares: Middleware[]; - environmentId?: number; + resolveEnvironmentId?: EnvironmentIdResolver; session?: OAuthSession; } -async function buildOAuthMiddlewares(config: BFFConfig, logger: Logger): Promise { +function buildOAuthMiddlewares(config: BFFConfig, logger: Logger): OAuthEdge { const oauthConfig = resolveOAuthConfig(config); if (!oauthConfig) { @@ -151,7 +153,7 @@ async function buildOAuthMiddlewares(config: BFFConfig, logger: Logger): Promise oauthConfig; const serverClient = new ForestServerClient({ forestServerUrl, envSecret: forestEnvSecret }); - const environmentId = await serverClient.fetchEnvironmentId(); + const resolveEnvironmentId = createEnvironmentIdResolver(serverClient); const sessionStore = createInMemorySessionStore({ cipher: createTokenCipher(tokenEncryptionKey), @@ -164,13 +166,13 @@ async function buildOAuthMiddlewares(config: BFFConfig, logger: Logger): Promise sessionStore, forestAppUrl, authSecret: forestAuthSecret, - environmentId, + resolveEnvironmentId, logger, }); return { middlewares: [oauthRoutes], - environmentId, + resolveEnvironmentId, session: { store: sessionStore, serverClient, forestServerUrl }, }; } @@ -318,7 +320,7 @@ function buildAgentRouteMiddlewares( } function buildAiMiddlewares(config: BFFConfig, oauth: OAuthEdge, logger: Logger): Middleware[] { - const { session, environmentId } = oauth; + const { session, resolveEnvironmentId } = oauth; if (!session) { logger('Warn', 'AI query route disabled: the deployment carries no OAuth session'); @@ -336,7 +338,7 @@ function buildAiMiddlewares(config: BFFConfig, oauth: OAuthEdge, logger: Logger) client, sessionStore: session.store, serverClient: session.serverClient, - environmentId, + resolveEnvironmentId, logger, }), ]; @@ -349,7 +351,6 @@ function buildAgentMiddlewares( aiMiddlewares: Middleware[], ): Middleware[] { const { forestAuthSecret, defaultTimezone } = config; - const { environmentId } = oauth; if (!forestAuthSecret) { logger('Warn', 'Agent edge disabled: FOREST_AUTH_SECRET is missing'); @@ -373,7 +374,16 @@ function buildAgentMiddlewares( source, hasAiQueryRoute: aiMiddlewares.length > 0, }), - ...(bundle ? [createContextRoutesMiddleware({ store: bundle.store, environmentId })] : []), + ...(bundle + ? [ + createContextRoutesMiddleware({ + store: bundle.store, + resolveEnvironmentId: oauth.resolveEnvironmentId + ? tolerateEnvironmentIdFailure(oauth.resolveEnvironmentId, logger) + : undefined, + }), + ] + : []), ...aiMiddlewares, createTimezoneMiddleware({ defaultTimezone }), ...buildAgentRouteMiddlewares(bundle, config, logger), @@ -397,7 +407,7 @@ export default async function buildBff({ }); } - const oauth = await buildOAuthMiddlewares(config, logger); + const oauth = buildOAuthMiddlewares(config, logger); const aiMiddlewares = buildAiMiddlewares(config, oauth, logger); const agentMiddlewares = buildAgentMiddlewares(config, logger, oauth, aiMiddlewares); const agentErrorMiddleware = diff --git a/packages/agent-bff/src/context/context-routes-middleware.ts b/packages/agent-bff/src/context/context-routes-middleware.ts index e3c131d6b1..954483f567 100644 --- a/packages/agent-bff/src/context/context-routes-middleware.ts +++ b/packages/agent-bff/src/context/context-routes-middleware.ts @@ -8,12 +8,16 @@ const CONTEXT_ROUTE = '/agent/v1/context'; export interface ContextRoutesMiddlewareOptions { store: ReadModelStore; - environmentId?: number; + /** + * Resolves to `undefined` rather than rejecting: the id decorates the payload, so a Forest server + * that cannot answer must not take the whole bootstrap route down with it. + */ + resolveEnvironmentId?: () => Promise; } export default function createContextRoutesMiddleware({ store, - environmentId, + resolveEnvironmentId, }: ContextRoutesMiddlewareOptions): Middleware { return async function contextRoutesMiddleware(ctx, next) { if (ctx.path !== CONTEXT_ROUTE || ctx.method !== 'GET') { @@ -23,6 +27,7 @@ export default function createContextRoutesMiddleware({ } const { collections, readModel, revision } = await resolveSchemaSnapshot(store); + const environmentId = await resolveEnvironmentId?.(); ctx.status = 200; ctx.body = buildContext(collections, readModel, { schemaRevision: revision, environmentId }); diff --git a/packages/agent-bff/src/http/bff-local-errors.ts b/packages/agent-bff/src/http/bff-local-errors.ts index 5dafd88646..7f5547ffac 100644 --- a/packages/agent-bff/src/http/bff-local-errors.ts +++ b/packages/agent-bff/src/http/bff-local-errors.ts @@ -89,3 +89,11 @@ export function actionRequiresApproval( ): BffHttpError { return new BffHttpError(403, 'action_requires_approval', message, { details }); } + +export function environmentUnresolved(): BffHttpError { + return new BffHttpError( + 502, + 'environment_unresolved', + 'The Forest environment could not be resolved', + ); +} diff --git a/packages/agent-bff/src/oauth/environment-fetch-error.ts b/packages/agent-bff/src/oauth/environment-fetch-error.ts new file mode 100644 index 0000000000..399e8e17a9 --- /dev/null +++ b/packages/agent-bff/src/oauth/environment-fetch-error.ts @@ -0,0 +1,38 @@ +/** Rate limiting is the one 4xx that heals on its own. */ +const RETRYABLE_CLIENT_STATUS = 429; + +function isPermanentStatus(status: number): boolean { + return status >= 400 && status < 500 && status !== RETRYABLE_CLIENT_STATUS; +} + +/** + * A failed environment id read, told apart by whether retrying can ever help: a Forest server that + * answers and refuses is a configuration error that will never resolve, anything else is transport. + * Consumers log the first at `Error` and the second at `Warn`, which is the only thing that lets an + * operator distinguish a wrong FOREST_ENV_SECRET from a SaaS blip. + */ +export default class EnvironmentFetchError extends Error { + readonly permanent: boolean; + + static fromStatus(status: number, statusText: string): EnvironmentFetchError { + return new EnvironmentFetchError( + `Failed to fetch environment: ${status} ${statusText}`, + isPermanentStatus(status), + ); + } + + constructor(message: string, permanent: boolean) { + super(message); + this.name = 'EnvironmentFetchError'; + this.permanent = permanent; + } +} + +/** Anything that is not a refusal from the Forest server — a socket error, a timeout — may heal. */ +export function isPermanentEnvironmentFailure(error: unknown): boolean { + return error instanceof EnvironmentFetchError && error.permanent; +} + +export function environmentFailureLevel(error: unknown): 'Error' | 'Warn' { + return isPermanentEnvironmentFailure(error) ? 'Error' : 'Warn'; +} diff --git a/packages/agent-bff/src/oauth/environment-id.ts b/packages/agent-bff/src/oauth/environment-id.ts new file mode 100644 index 0000000000..dbf411c421 --- /dev/null +++ b/packages/agent-bff/src/oauth/environment-id.ts @@ -0,0 +1,85 @@ +import type ForestServerClient from './forest-server-client'; +import type { Logger } from '../ports/logger-port'; + +import { environmentFailureLevel } from './environment-fetch-error'; + +/** Resolves the Forest environment id, on demand rather than at boot. */ +export type EnvironmentIdResolver = () => Promise; + +/** + * How long a failure is remembered. Short enough that a transient outage still heals on the next + * request — the whole point of resolving lazily — long enough that a burst of requests costs the + * Forest server one round trip instead of one each, and costs the caller one deadline instead of + * one per request. + */ +const FAILURE_TTL_MS = 5_000; + +/** + * Fetch the environment id at most once per success, and hold a failure only for `FAILURE_TTL_MS`: + * a Forest server that is briefly unreachable must not disable the routes that need the id for the + * life of the process, which is what resolving it at boot did. Concurrent callers share the + * in-flight fetch. + * + * The three consumers deliberately disagree on what a failure means, because the id is worth a + * different amount to each: `/oauth/authorize` cannot mint a login without it and surfaces + * `server_error`, the AI relay cannot address the proxy without it and refuses the query, and + * `/agent/v1/context` merely decorates its payload with it and drops the field (see + * `tolerateEnvironmentIdFailure`). + */ +export default function createEnvironmentIdResolver( + client: Pick, + now: () => number = Date.now, +): EnvironmentIdResolver { + let environmentId: number | undefined; + let inFlight: Promise | null = null; + let failedAt: number | undefined; + let failure: unknown; + + return async function resolveEnvironmentId(): Promise { + if (environmentId !== undefined) return environmentId; + if (failedAt !== undefined && now() - failedAt < FAILURE_TTL_MS) throw failure; + + inFlight ??= client + .fetchEnvironmentId() + .then(resolved => { + environmentId = resolved; + failedAt = undefined; + + return resolved; + }) + .catch(error => { + failedAt = now(); + failure = error; + + throw error; + }) + .finally(() => { + inFlight = null; + }); + + return inFlight; + }; +} + +/** + * The context payload carries the environment id when there is one, and drops it when the Forest + * server cannot be reached — the route itself must keep answering either way. A refusal from the + * Forest server is logged at `Error` because it will never resolve on its own; a transport failure + * stays a `Warn`. + */ +export function tolerateEnvironmentIdFailure( + resolveEnvironmentId: EnvironmentIdResolver, + logger: Logger, +): () => Promise { + return async function resolveOrForget(): Promise { + try { + return await resolveEnvironmentId(); + } catch (error) { + logger(environmentFailureLevel(error), 'Serving the context without an environment id', { + cause: error instanceof Error ? error.message : String(error), + }); + + return undefined; + } + }; +} diff --git a/packages/agent-bff/src/oauth/forest-server-client.ts b/packages/agent-bff/src/oauth/forest-server-client.ts index 06fd7989e1..ad3accdaf8 100644 --- a/packages/agent-bff/src/oauth/forest-server-client.ts +++ b/packages/agent-bff/src/oauth/forest-server-client.ts @@ -3,9 +3,10 @@ import type { UserInfo } from '@forestadmin/forestadmin-client'; import createForestAdminClient from '@forestadmin/forestadmin-client'; import jsonwebtoken from 'jsonwebtoken'; +import EnvironmentFetchError from './environment-fetch-error'; import OAuthExchangeError from './oauth-exchange-error'; -export { OAuthExchangeError }; +export { EnvironmentFetchError, OAuthExchangeError }; export interface RegisteredClient { client_id: string; @@ -36,8 +37,20 @@ export interface ForestServerClientOptions { const DEFAULT_HEADERS = { 'Content-Type': 'application/json' } as const; const REQUEST_TIMEOUT_MS = 60_000; -function fetchWithTimeout(url: string, init: RequestInit): Promise { - return fetch(url, { ...init, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS) }); +/** + * The environment id sits on the critical path of routes that only decorate their payload with it, + * and it is a configuration read rather than a user operation: it gets a deadline of its own, short + * enough that a Forest server which accepts the connection and then never answers costs seconds + * instead of a minute per request. + */ +const ENVIRONMENT_TIMEOUT_MS = 5_000; + +function fetchWithTimeout( + url: string, + init: RequestInit, + timeoutMs = REQUEST_TIMEOUT_MS, +): Promise { + return fetch(url, { ...init, signal: AbortSignal.timeout(timeoutMs) }); } export default class ForestServerClient { @@ -56,26 +69,36 @@ export default class ForestServerClient { } async fetchEnvironmentId(): Promise { - const response = await fetchWithTimeout(this.url('/liana/environment'), { - method: 'GET', - headers: { ...DEFAULT_HEADERS, 'forest-secret-key': this.envSecret }, - }); + const response = await fetchWithTimeout( + this.url('/liana/environment'), + { + method: 'GET', + headers: { ...DEFAULT_HEADERS, 'forest-secret-key': this.envSecret }, + }, + ENVIRONMENT_TIMEOUT_MS, + ); if (!response.ok) { - throw new Error(`Failed to fetch environment: ${response.status} ${response.statusText}`); + throw EnvironmentFetchError.fromStatus(response.status, response.statusText); } const body = (await response.json()) as { data?: { id?: string | number } }; const id = body.data?.id; if (typeof id !== 'number' && (typeof id !== 'string' || !/^\d+$/.test(id))) { - throw new Error('Failed to parse environment id from the Forest server response'); + throw new EnvironmentFetchError( + 'Failed to parse environment id from the Forest server response', + true, + ); } const environmentId = Number(id); if (!Number.isInteger(environmentId) || environmentId <= 0) { - throw new Error('Failed to parse environment id from the Forest server response'); + throw new EnvironmentFetchError( + 'Failed to parse environment id from the Forest server response', + true, + ); } return environmentId; diff --git a/packages/agent-bff/src/oauth/oauth-routes.ts b/packages/agent-bff/src/oauth/oauth-routes.ts index f1c170b6b4..61e55d46f3 100644 --- a/packages/agent-bff/src/oauth/oauth-routes.ts +++ b/packages/agent-bff/src/oauth/oauth-routes.ts @@ -1,3 +1,4 @@ +import type { EnvironmentIdResolver } from './environment-id'; import type ForestServerClient from './forest-server-client'; import type { ServerTokens } from './forest-server-client'; import type { SessionStore } from './session-store'; @@ -16,6 +17,7 @@ import { invalidClient, invalidGrant, invalidRequest, + serverError, sessionExpired, sessionInvalidated, toErrorBody, @@ -32,7 +34,7 @@ export interface OAuthRoutesOptions { sessionStore: SessionStore; forestAppUrl: string; authSecret: string; - environmentId: number; + resolveEnvironmentId: EnvironmentIdResolver; logger: Logger; } @@ -110,6 +112,24 @@ function mapIdentityError(error: unknown): OAuthRequestError { return new OAuthRequestError(502, 'identity_resolution_failed', 'Failed to resolve identity'); } +/** + * The id is only needed once the redirect_uri is trusted, so a resolution failure travels back to + * the client as `server_error` (RFC 6749 §4.1.2.1) instead of a bare 500. Logged at `Error` whatever + * the cause: the route still answers 302, so no status-code alert can fire, and every login is + * broken until the id resolves. + */ +async function resolveEnvironmentId(options: OAuthRoutesOptions): Promise { + try { + return await options.resolveEnvironmentId(); + } catch (error) { + options.logger('Error', 'Could not resolve the Forest environment id', { + cause: error instanceof Error ? error.message : String(error), + }); + + throw serverError('Could not resolve the Forest environment', error); + } +} + function redirectAuthorizeError( ctx: Context, redirectUri: string, @@ -181,7 +201,7 @@ async function handleAuthorize(ctx: Context, options: OAuthRoutesOptions): Promi authorizeUrl.searchParams.set('code_challenge', codeChallenge); authorizeUrl.searchParams.set('code_challenge_method', 'S256'); authorizeUrl.searchParams.set('state', state); - authorizeUrl.searchParams.set('environmentId', String(options.environmentId)); + authorizeUrl.searchParams.set('environmentId', String(await resolveEnvironmentId(options))); ctx.redirect(authorizeUrl.toString()); } catch (error) { diff --git a/packages/agent-bff/test/ai/ai-routes-middleware.test.ts b/packages/agent-bff/test/ai/ai-routes-middleware.test.ts index 79f90bbf46..331f8a9f64 100644 --- a/packages/agent-bff/test/ai/ai-routes-middleware.test.ts +++ b/packages/agent-bff/test/ai/ai-routes-middleware.test.ts @@ -12,6 +12,7 @@ import RealAiProxyClient, { AiProxyTimeoutError } from '../../src/ai/ai-proxy-cl import createAiRoutesMiddleware, { AI_QUERY_ROUTE } from '../../src/ai/ai-routes-middleware'; import { AI_BODY_LIMIT } from '../../src/http/body-limit'; import createErrorMiddleware from '../../src/http/error-middleware'; +import EnvironmentFetchError from '../../src/oauth/environment-fetch-error'; import { restoreFetchAfterEach, stubFetch } from '../helpers/fetch-stub'; const SAAS_ACCESS_TOKEN = 'saas-access-token'; @@ -43,6 +44,7 @@ interface AppContext { authMode?: 'oauth' | 'api-key'; renderingId?: string; environmentId?: number; + resolveEnvironmentId?: () => Promise; } function buildApp( @@ -53,6 +55,7 @@ function buildApp( authMode = 'oauth', renderingId = '42', environmentId = 7, + resolveEnvironmentId = async () => environmentId, }: AppContext, ) { const sessionStore = store ?? makeSessionStore({ saasAccessToken: freshAccessToken() }); @@ -70,7 +73,15 @@ function buildApp( await next(); }); - app.use(createAiRoutesMiddleware({ client, sessionStore, serverClient, environmentId, logger })); + app.use( + createAiRoutesMiddleware({ + client, + sessionStore, + serverClient, + resolveEnvironmentId, + logger, + }), + ); return { app, sessionStore, logger }; } @@ -175,6 +186,56 @@ describe('createAiRoutesMiddleware', () => { }); }); + describe('when the Forest server refuses the environment id read', () => { + it('should answer 502 environment_unresolved and never contact the AI proxy', async () => { + const { app, query } = makeApp({ + resolveEnvironmentId: async () => { + throw EnvironmentFetchError.fromStatus(401, 'Unauthorized'); + }, + }); + + const response = await request(app.callback()).post(AI_QUERY_ROUTE).send({ messages: [] }); + + expect(response.status).toBe(502); + expect(response.body.error.type).toBe('environment_unresolved'); + expect(query).not.toHaveBeenCalled(); + }); + + it('should log at Error naming the environment read, not the AI query', async () => { + const { app, logger } = makeApp({ + resolveEnvironmentId: async () => { + throw EnvironmentFetchError.fromStatus(401, 'Unauthorized'); + }, + }); + + await request(app.callback()).post(AI_QUERY_ROUTE).send({ messages: [] }); + + expect(logger).toHaveBeenCalledWith( + 'Error', + 'AI query refused: the Forest environment id could not be resolved', + { cause: 'EnvironmentFetchError: Failed to fetch environment: 401 Unauthorized' }, + ); + }); + }); + + describe('when the environment id read times out', () => { + it('should answer 504 upstream_timeout rather than a network error', async () => { + const timeout = new Error('The operation was aborted due to timeout'); + timeout.name = 'TimeoutError'; + const { app, query } = makeApp({ + resolveEnvironmentId: async () => { + throw timeout; + }, + }); + + const response = await request(app.callback()).post(AI_QUERY_ROUTE).send({ messages: [] }); + + expect(response.status).toBe(504); + expect(response.body.error.type).toBe('upstream_timeout'); + expect(query).not.toHaveBeenCalled(); + }); + }); + describe('when the session is valid', () => { it('should forward the session access token, not the incoming Authorization header', async () => { const saasAccessToken = freshAccessToken(); diff --git a/packages/agent-bff/test/cli-core.test.ts b/packages/agent-bff/test/cli-core.test.ts index b03dd472a0..1f414be9df 100644 --- a/packages/agent-bff/test/cli-core.test.ts +++ b/packages/agent-bff/test/cli-core.test.ts @@ -113,7 +113,6 @@ describe('runCli', () => { } satisfies NodeJS.ProcessEnv; it('should wire OAuth routes (no disabled warning) and boot', async () => { - const fetchEnvironmentId = stubEnvironmentIdFetch(); const logs: string[] = []; const logger: Logger = (_level, message) => { @@ -125,20 +124,35 @@ describe('runCli', () => { try { expect(server).toBeDefined(); expect(logs).not.toContain('OAuth routes disabled: required configuration is missing'); - expect(fetchEnvironmentId).toHaveBeenCalledTimes(1); } finally { await server.stop(); } }); - it('should propagate a fetchEnvironmentId failure out of runCli', async () => { + it('should not reach the Forest server while booting', async () => { + const fetchEnvironmentId = stubEnvironmentIdFetch(); + + const server = await runCli({ ...FULL_ENV }, noopLogger); + + try { + expect(fetchEnvironmentId).not.toHaveBeenCalled(); + } finally { + await server.stop(); + } + }); + + it('should still boot when the Forest server is unreachable', async () => { global.fetch = jest .fn() .mockRejectedValue(new Error('forest server unreachable')) as unknown as typeof fetch; - await expect(runCli({ ...FULL_ENV }, noopLogger)).rejects.toThrow( - 'forest server unreachable', - ); + const server = await runCli({ ...FULL_ENV }, noopLogger); + + try { + expect(server).toBeDefined(); + } finally { + await server.stop(); + } }); }); diff --git a/packages/agent-bff/test/context/context-routes-middleware.test.ts b/packages/agent-bff/test/context/context-routes-middleware.test.ts index 8090e3c391..d4b217c115 100644 --- a/packages/agent-bff/test/context/context-routes-middleware.test.ts +++ b/packages/agent-bff/test/context/context-routes-middleware.test.ts @@ -11,6 +11,7 @@ import createAuthModeMiddleware from '../../src/auth/auth-mode-middleware'; import createContextRoutesMiddleware from '../../src/context/context-routes-middleware'; import createPerKeyOriginMiddleware from '../../src/cors/per-key-origin'; import createErrorMiddleware from '../../src/http/error-middleware'; +import { tolerateEnvironmentIdFailure } from '../../src/oauth/environment-id'; import CapabilitiesCache from '../../src/read-model/capabilities-cache'; import ReadModelStore from '../../src/read-model/read-model-store'; import SchemaCache, { ONE_DAY_MS } from '../../src/read-model/schema-cache'; @@ -32,7 +33,9 @@ function makeRouteOnlyApp(fetchSchema: jest.Mock, environmentId?: number, now?: const app = new Koa(); app.use(createErrorMiddleware({ logger: () => {} })); - app.use(createContextRoutesMiddleware({ store, environmentId })); + app.use( + createContextRoutesMiddleware({ store, resolveEnvironmentId: async () => environmentId }), + ); return { app, schemaCache }; } @@ -102,7 +105,7 @@ describe('contextRoutesMiddleware', () => { expect(response.body.meta).toEqual({ schemaRevision: 1 }); }); - it('should carry the environment id the deployment resolved at boot', async () => { + it('should carry the environment id the resolver returns', async () => { const { app } = makeRouteOnlyApp(jest.fn().mockResolvedValue(schema), 42); const response = await request(app.callback()).get(ROUTE); @@ -110,7 +113,7 @@ describe('contextRoutesMiddleware', () => { expect(response.body.meta).toEqual({ schemaRevision: 1, environmentId: 42 }); }); - it('should omit the environment id when the deployment resolved none', async () => { + it('should omit the environment id when the resolver returns none', async () => { const { app } = makeRouteOnlyApp(jest.fn().mockResolvedValue(schema)); const response = await request(app.callback()).get(ROUTE); @@ -118,6 +121,38 @@ describe('contextRoutesMiddleware', () => { expect(response.body.meta).toEqual({ schemaRevision: 1 }); }); + it('should still answer the contract when the environment id cannot be resolved', async () => { + const logs: { level: string; message: string; context?: unknown }[] = []; + const { store } = makeStore(jest.fn().mockResolvedValue(schema)); + + const app = new Koa(); + app.use(createErrorMiddleware({ logger: () => {} })); + app.use( + createContextRoutesMiddleware({ + store, + resolveEnvironmentId: tolerateEnvironmentIdFailure( + async () => { + throw new Error('forest server unreachable'); + }, + (level, message, context) => logs.push({ level, message, context }), + ), + }), + ); + + const response = await request(app.callback()).get(ROUTE); + + expect(response.status).toBe(200); + expect(response.body.meta).toEqual({ schemaRevision: 1 }); + expect(response.body.collections).toHaveLength(4); + expect(logs).toEqual([ + { + level: 'Warn', + message: 'Serving the context without an environment id', + context: { cause: 'forest server unreachable' }, + }, + ]); + }); + it('should fetch the schema once on a cold cache and never again while it stays warm', async () => { const fetchSchema = jest.fn().mockResolvedValue(schema); const { app } = makeRouteOnlyApp(fetchSchema); diff --git a/packages/agent-bff/test/oauth/environment-id.test.ts b/packages/agent-bff/test/oauth/environment-id.test.ts new file mode 100644 index 0000000000..e495f97e5f --- /dev/null +++ b/packages/agent-bff/test/oauth/environment-id.test.ts @@ -0,0 +1,133 @@ +import type { Logger } from '../../src/ports/logger-port'; + +import EnvironmentFetchError from '../../src/oauth/environment-fetch-error'; +import createEnvironmentIdResolver, { + tolerateEnvironmentIdFailure, +} from '../../src/oauth/environment-id'; + +const FAILURE_TTL_MS = 5_000; + +function makeLogger() { + const logs: { level: string; message: string; context?: unknown }[] = []; + + const logger: Logger = (level, message, context) => { + logs.push({ level, message, context }); + }; + + return { logger, logs }; +} + +describe('createEnvironmentIdResolver', () => { + describe('when the fetch succeeds', () => { + it('should fetch once and serve the cached id afterwards', async () => { + const fetchEnvironmentId = jest.fn().mockResolvedValue(42); + const resolve = createEnvironmentIdResolver({ fetchEnvironmentId }); + + await expect(resolve()).resolves.toBe(42); + await expect(resolve()).resolves.toBe(42); + + expect(fetchEnvironmentId).toHaveBeenCalledTimes(1); + }); + }); + + describe('when the fetch fails', () => { + it('should serve the remembered failure without refetching for the whole failure ttl', async () => { + const fetchEnvironmentId = jest + .fn() + .mockRejectedValue(new Error('forest server unreachable')); + let clock = 1_000_000; + const resolve = createEnvironmentIdResolver({ fetchEnvironmentId }, () => clock); + + await expect(resolve()).rejects.toThrow('forest server unreachable'); + clock += FAILURE_TTL_MS - 1; + await expect(resolve()).rejects.toThrow('forest server unreachable'); + + expect(fetchEnvironmentId).toHaveBeenCalledTimes(1); + }); + + it('should retry and resolve once the failure ttl has elapsed, since the failure is not cached for good', async () => { + const fetchEnvironmentId = jest + .fn() + .mockRejectedValueOnce(new Error('forest server unreachable')) + .mockResolvedValue(7); + let clock = 1_000_000; + const resolve = createEnvironmentIdResolver({ fetchEnvironmentId }, () => clock); + + await expect(resolve()).rejects.toThrow('forest server unreachable'); + clock += FAILURE_TTL_MS; + + await expect(resolve()).resolves.toBe(7); + expect(fetchEnvironmentId).toHaveBeenCalledTimes(2); + }); + }); + + describe('when two callers arrive before the first fetch settles', () => { + it('should share a single fetch', async () => { + const fetchEnvironmentId = jest.fn().mockResolvedValue(1); + const resolve = createEnvironmentIdResolver({ fetchEnvironmentId }); + + await expect(Promise.all([resolve(), resolve()])).resolves.toEqual([1, 1]); + + expect(fetchEnvironmentId).toHaveBeenCalledTimes(1); + }); + }); +}); + +describe('tolerateEnvironmentIdFailure', () => { + describe('when the resolver succeeds', () => { + it('should serve the id untouched and log nothing', async () => { + const { logger, logs } = makeLogger(); + + await expect(tolerateEnvironmentIdFailure(async () => 42, logger)()).resolves.toBe(42); + expect(logs).toEqual([]); + }); + }); + + describe('when the Forest server refuses the read', () => { + it('should serve no id and log at Error, since a refusal will never resolve on its own', async () => { + const { logger, logs } = makeLogger(); + const resolve = tolerateEnvironmentIdFailure(async () => { + throw EnvironmentFetchError.fromStatus(401, 'Unauthorized'); + }, logger); + + await expect(resolve()).resolves.toBeUndefined(); + expect(logs).toEqual([ + { + level: 'Error', + message: 'Serving the context without an environment id', + context: { cause: 'Failed to fetch environment: 401 Unauthorized' }, + }, + ]); + }); + }); + + describe('when the Forest server cannot be reached', () => { + it('should serve no id and log at Warn, since transport failures heal', async () => { + const { logger, logs } = makeLogger(); + const resolve = tolerateEnvironmentIdFailure(async () => { + throw new Error('forest server unreachable'); + }, logger); + + await expect(resolve()).resolves.toBeUndefined(); + expect(logs).toEqual([ + { + level: 'Warn', + message: 'Serving the context without an environment id', + context: { cause: 'forest server unreachable' }, + }, + ]); + }); + }); + + describe('when a rate limit is what refused the read', () => { + it('should log at Warn, since 429 is the one 4xx that heals', async () => { + const { logger, logs } = makeLogger(); + const resolve = tolerateEnvironmentIdFailure(async () => { + throw EnvironmentFetchError.fromStatus(429, 'Too Many Requests'); + }, logger); + + await expect(resolve()).resolves.toBeUndefined(); + expect(logs[0].level).toBe('Warn'); + }); + }); +}); diff --git a/packages/agent-bff/test/oauth/oauth-routes.test.ts b/packages/agent-bff/test/oauth/oauth-routes.test.ts index 71432ef22d..2ceef09962 100644 --- a/packages/agent-bff/test/oauth/oauth-routes.test.ts +++ b/packages/agent-bff/test/oauth/oauth-routes.test.ts @@ -85,6 +85,7 @@ function buildApp( serverClient: ForestServerClient, forestAppUrl: string = APP_URL, onTokenRequest?: () => void, + resolveEnvironmentId: () => Promise = async () => 99, ) { const logs: LogLine[] = []; @@ -115,7 +116,7 @@ function buildApp( sessionStore, forestAppUrl, authSecret: AUTH_SECRET, - environmentId: 99, + resolveEnvironmentId, logger, }), ); @@ -156,6 +157,25 @@ describe('oauth-routes GET /oauth/authorize', () => { expect(location.searchParams.get('state')).toBe('state-xyz'); }); + it('should redirect server_error to the client when the environment id cannot be resolved', async () => { + const { app, logs } = buildApp(stubServerClient(), APP_URL, undefined, async () => { + throw new Error('forest server unreachable'); + }); + + const response = await request(app.callback()).get('/oauth/authorize').query(AUTHORIZE_QUERY); + + expect(response.status).toBe(302); + const location = new URL(response.headers.location); + expect(location.origin + location.pathname).toBe(REDIRECT_URI); + expect(location.searchParams.get('error')).toBe('server_error'); + expect(location.searchParams.get('state')).toBe('state-xyz'); + expect(logs).toContainEqual({ + level: 'Error', + message: 'Could not resolve the Forest environment id', + context: { cause: 'forest server unreachable' }, + }); + }); + it('should preserve a path prefix already present in the Forest app url', async () => { const { app } = buildApp(stubServerClient(), 'https://app.forestadmin.com/prefix'); @@ -610,7 +630,7 @@ describe('oauth-routes POST /oauth/token', () => { sessionStore, forestAppUrl: APP_URL, authSecret: AUTH_SECRET, - environmentId: 99, + resolveEnvironmentId: async () => 99, logger: () => undefined, }), );