diff --git a/.changeset/adr-0097-openapi-upstream-classify.md b/.changeset/adr-0097-openapi-upstream-classify.md new file mode 100644 index 0000000000..0c6cbb0d59 --- /dev/null +++ b/.changeset/adr-0097-openapi-upstream-classify.md @@ -0,0 +1,20 @@ +--- +'@objectstack/connector-openapi': minor +--- + +feat(connector-openapi): degrade + retry on an unreachable remote spec URL (#3049 follow-up) + +The `openapi` provider fetches `providerConfig.spec` when it is an http(s) URL. +That fetch previously threw plain on any failure, so a momentarily-unreachable +spec endpoint aborted the whole app boot. It now classifies the fault the same +way `connector-mcp` classifies its connect path (ADR-0097): + +- **Network error** (DNS / connection refused / timeout) or a **transient HTTP + status** (`408` / `429` / `5xx`, mirroring the `retryableStatusCodes` + convention) throws `ConnectorUpstreamUnavailableError` — the materializer + degrades the instance (`state: 'degraded'` on `GET /connectors`, dispatch + fails clearly) and retries with backoff plus on every `metadata:reloaded`. +- A **wrong URL** (non-retryable `4xx`) or an **unparseable document** stays a + plain, fatal configuration fault. + +Inline and file-path (`#3016`) specs do no boot I/O and are unaffected. diff --git a/docs/adr/0097-declarative-connector-instances.md b/docs/adr/0097-declarative-connector-instances.md index e44926051e..d169c9ea91 100644 --- a/docs/adr/0097-declarative-connector-instances.md +++ b/docs/adr/0097-declarative-connector-instances.md @@ -167,6 +167,13 @@ dispatch was rejected: MCP actions ARE the server's `tools/list`, so a connector materialized without connecting would register a zero-action def — exactly the plausible-but-dead shape this ADR exists to kill. +The `openapi` provider applies the same split to its **remote-URL spec fetch**: +an unreachable endpoint (network error) or a transient HTTP status +(`408` / `429` / `5xx`, mirroring the `retryableStatusCodes` convention) is +upstream-unavailable — degrade + retry — while a wrong URL (non-retryable +`4xx`) or an unparseable document stays a fatal configuration fault. Inline and +file-path (`#3016`) specs do no boot I/O and are unaffected. + ### Declarative stdio policy: default-deny (follow-up, landed — #3055) A declarative `provider: 'mcp'` entry may name a **stdio transport**, and @@ -196,5 +203,4 @@ any default preset (#3056). - **`providerConfig.spec` (openapi)** accepts an inline document, an http(s) URL, **or a package-relative file path** (#3016 follow-up). The connector still owns no filesystem access: the automation service injects a `loadPackageFile` capability into `ConnectorProviderContext` that resolves the ref against the declaring stack/package root (`packageRoot`, CLI default: the `objectstack.config.ts` directory) and **confines reads to that root** — absolute and `..`-escaping paths are rejected. Read/parse failures follow the reconcile policy above: fatal at boot, skipped on reload. - **MCP credentials** ride the transport (ADR-0024); for an http transport a resolved `auth` is folded into the request headers. -- **A live `provider: 'mcp'` showcase demo** remains deferred: materialization needs a reachable MCP server, and the natural in-repo target (`@objectstack/mcp`, the platform's own MCP endpoint) is not listening until after the automation plugin's `start()` — the demo would deterministically boot degraded and heal seconds later, making the dogfood CI gate timing-sensitive. It should land together with an in-repo MCP fixture server (or an explicit boot-ordering story for self-connection). -- **The openapi provider's remote-URL spec fetch** still throws plain on network failure (fatal at boot). It can adopt the same `CONNECTOR_UPSTREAM_UNAVAILABLE` classification once operators ask for it — the mechanism is provider-agnostic. +- **A live `provider: 'mcp'` showcase demo** landed with #3056 (#3062): the showcase points a declarative `mcp` instance at an in-repo stdio MCP fixture (`examples/app-showcase/scripts/mcp-fixture.mjs`), spawned under the host `declarativeStdio` allowlist, and a dogfood proof pins materialization + dispatch. Self-connection to the platform's own `@objectstack/mcp` endpoint (which is not listening until after the automation plugin's `start()`) stays out of scope — the fixture sidesteps the boot-ordering timing entirely. diff --git a/packages/connectors/connector-openapi/src/openapi-provider.test.ts b/packages/connectors/connector-openapi/src/openapi-provider.test.ts index 07bee09afd..6b75343398 100644 --- a/packages/connectors/connector-openapi/src/openapi-provider.test.ts +++ b/packages/connectors/connector-openapi/src/openapi-provider.test.ts @@ -5,6 +5,7 @@ import { describe, it, expect } from 'vitest'; import type { ConnectorProviderContext } from '@objectstack/spec/integration'; +import { isConnectorUpstreamUnavailable } from '@objectstack/spec/integration'; import { createOpenApiProviderFactory, OPENAPI_PROVIDER_KEY } from './openapi-provider.js'; const petstore = { @@ -96,3 +97,73 @@ describe('openapi provider factory (ADR-0097)', () => { ).rejects.toThrow(/no package file access/); }); }); + +// ── #3049 follow-up: remote-URL fetch fault classification ────────────────── +// +// A remote spec URL that is unreachable / transiently failing is an OPERATIONAL +// fault: the factory throws the CONNECTOR_UPSTREAM_UNAVAILABLE marker so the +// materializer degrades + retries the instance instead of failing boot +// (symmetric with connector-mcp's connect path). A wrong URL (non-retryable +// 4xx) or an unparseable document stays a plain, fatal configuration fault. + +describe('openapi provider — remote spec fetch fault classification (#3049)', () => { + const url = 'https://petstore.example.com/openapi.json'; + const cfg = { providerConfig: { spec: url } }; + + it('classifies a network failure (fetch rejects) as upstream-unavailable, keeping the cause', async () => { + const boom = new Error('connect ECONNREFUSED 93.184.216.34:443'); + const fetchImpl = (async () => { throw boom; }) as unknown as typeof fetch; + const factory = createOpenApiProviderFactory({ fetchImpl }); + + const err = await factory(ctx(cfg)).then( + () => { throw new Error('expected rejection'); }, + (e: unknown) => e, + ); + expect(isConnectorUpstreamUnavailable(err)).toBe(true); + expect((err as Error).message).toMatch(/'pets' could not reach spec URL/); + expect((err as Error).message).toContain('ECONNREFUSED'); + expect((err as { cause?: unknown }).cause).toBe(boom); + }); + + it.each([408, 429, 500, 502, 503, 504])( + 'classifies a transient HTTP %i as upstream-unavailable (retryable)', + async (status) => { + const fetchImpl = (async () => ({ ok: false, status, json: async () => ({}) }) as unknown as Response) as unknown as typeof fetch; + const factory = createOpenApiProviderFactory({ fetchImpl }); + const err = await factory(ctx(cfg)).then(() => { throw new Error('expected rejection'); }, (e: unknown) => e); + expect(isConnectorUpstreamUnavailable(err), `HTTP ${status} should degrade`).toBe(true); + expect((err as Error).message).toContain(String(status)); + }, + ); + + it.each([400, 401, 403, 404, 410])( + 'keeps a non-retryable HTTP %i a plain (fatal) configuration fault', + async (status) => { + const fetchImpl = (async () => ({ ok: false, status, json: async () => ({}) }) as unknown as Response) as unknown as typeof fetch; + const factory = createOpenApiProviderFactory({ fetchImpl }); + const err = await factory(ctx(cfg)).then(() => { throw new Error('expected rejection'); }, (e: unknown) => e); + expect(isConnectorUpstreamUnavailable(err), `HTTP ${status} should stay fatal`).toBe(false); + expect((err as Error).message).toMatch(/failed to fetch spec/); + }, + ); + + it('keeps a 2xx-but-unparseable body a plain (fatal) content fault, not upstream-unavailable', async () => { + const fetchImpl = (async () => ({ + ok: true, + status: 200, + json: async () => { throw new SyntaxError('Unexpected token < in JSON'); }, + }) as unknown as Response) as unknown as typeof fetch; + const factory = createOpenApiProviderFactory({ fetchImpl }); + const err = await factory(ctx(cfg)).then(() => { throw new Error('expected rejection'); }, (e: unknown) => e); + expect(isConnectorUpstreamUnavailable(err)).toBe(false); + expect((err as Error).message).toMatch(/not a parseable.*OpenAPI JSON document/s); + }); + + it('keeps a 2xx non-object body (array) a plain (fatal) content fault', async () => { + const fetchImpl = (async () => ({ ok: true, status: 200, json: async () => [1, 2] }) as unknown as Response) as unknown as typeof fetch; + const factory = createOpenApiProviderFactory({ fetchImpl }); + const err = await factory(ctx(cfg)).then(() => { throw new Error('expected rejection'); }, (e: unknown) => e); + expect(isConnectorUpstreamUnavailable(err)).toBe(false); + expect((err as Error).message).toMatch(/not a parseable.*OpenAPI JSON document/s); + }); +}); diff --git a/packages/connectors/connector-openapi/src/openapi-provider.ts b/packages/connectors/connector-openapi/src/openapi-provider.ts index 9def34727e..a2ffe961b1 100644 --- a/packages/connectors/connector-openapi/src/openapi-provider.ts +++ b/packages/connectors/connector-openapi/src/openapi-provider.ts @@ -5,6 +5,7 @@ import type { ConnectorProviderFactory, ResolvedConnectorAuth, } from '@objectstack/spec/integration'; +import { ConnectorUpstreamUnavailableError } from '@objectstack/spec/integration'; import { createOpenApiConnector, type OpenApiDocument, @@ -17,6 +18,17 @@ import { */ export const OPENAPI_PROVIDER_KEY = 'openapi'; +/** + * HTTP statuses treated as **transient** when fetching a remote spec at boot + * (#3049 follow-up): a timeout / rate-limit / 5xx means the spec endpoint is + * momentarily unhealthy, not that the connector is misconfigured — so the + * instance degrades and retries rather than aborting boot. Mirrors the + * connector request-retry convention (`retryableStatusCodes` default in + * `ConnectorSchema`). Any OTHER non-2xx (400 / 401 / 403 / 404 / 410 …) means + * the request itself is wrong — a configuration fault that stays fatal. + */ +const SPEC_FETCH_RETRYABLE_STATUS = new Set([408, 429, 500, 502, 503, 504]); + /** Injectable dependencies for {@link createOpenApiProviderFactory} (tests). */ export interface OpenApiProviderDeps { /** Injected fetch implementation (spec fetch + request transport); defaults to global `fetch`. */ @@ -29,6 +41,10 @@ interface OpenApiProviderConfig { * The OpenAPI 3.x document: an inline object, an http(s) URL to fetch at * boot, or a file path resolved relative to the declaring stack/package root * (`'./billing-openapi.json'`, #3016). + * + * A remote URL that is unreachable / transiently failing (network error, + * 408 / 429 / 5xx) degrades the instance and retries (#3049); a wrong URL + * (non-retryable 4xx) or an unparseable document stays a fatal config fault. */ spec?: unknown; /** Override the base URL (else the document's `servers[0].url`). */ @@ -41,9 +57,14 @@ interface OpenApiProviderConfig { * form used by the showcase), an http(s) URL fetched at materialization, or a * **file path** read through the host's `ctx.loadPackageFile` — which resolves * it relative to the declaring stack/package root and confines the read to - * that root (absolute / `..`-escaping paths are rejected there). Every failure - * throws, so the materializer's reconcile policy applies: fatal at boot, the - * entry is skipped on reload. + * that root (absolute / `..`-escaping paths are rejected there). + * + * Fault classification (#3049 seam, symmetric with connector-mcp's connect + * path): a remote spec URL that is unreachable or transiently failing throws + * {@link ConnectorUpstreamUnavailableError} — the materializer degrades the + * instance and retries. Every OTHER failure (missing spec, a wrong URL, + * an unparseable document, no host file access) throws a plain error, which + * stays fatal at boot / a skipped entry on reload. */ async function loadOpenApiDocument( spec: unknown, @@ -57,13 +78,48 @@ async function loadOpenApiDocument( if (typeof spec === 'string' && spec.length > 0) { if (/^https?:\/\//i.test(spec)) { const doFetch = fetchImpl ?? fetch; - const res = await doFetch(spec); + let res: Response; + try { + res = await doFetch(spec); + } catch (err) { + // The spec endpoint is unreachable — DNS / connection refused / + // timeout / network error. Operational, not a config mistake: degrade + // + retry rather than fail boot. + throw new ConnectorUpstreamUnavailableError( + `connector-openapi provider: connector '${connectorName}' could not reach spec URL '${spec}': ${(err as Error).message}`, + { cause: err }, + ); + } if (!res.ok) { + if (SPEC_FETCH_RETRYABLE_STATUS.has(res.status)) { + // A transient server-side status (timeout / rate-limit / 5xx) — the + // endpoint is momentarily unhealthy, so treat it as upstream-unavailable. + throw new ConnectorUpstreamUnavailableError( + `connector-openapi provider: connector '${connectorName}' got a transient HTTP ${res.status} fetching spec '${spec}'.`, + ); + } + // A non-retryable status (400 / 401 / 403 / 404 / 410 …) — the request + // is wrong (bad URL, missing/insufficient auth for the spec endpoint): + // a configuration fault that stays fatal. throw new Error( `connector-openapi provider: connector '${connectorName}' failed to fetch spec '${spec}' (HTTP ${res.status}).`, ); } - return (await res.json()) as OpenApiDocument; + // A 2xx with an unparseable / non-object body is a content fault (the + // endpoint served the wrong thing) — fatal, symmetric with the file-path + // parse below. + try { + const parsed: unknown = await res.json(); + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error('not a JSON object'); + } + return parsed as OpenApiDocument; + } catch (err) { + throw new Error( + `connector-openapi provider: connector '${connectorName}' fetched spec '${spec}' but it is not a parseable ` + + `OpenAPI JSON document: ${(err as Error).message}`, + ); + } } // File path — dereferenced through the host capability so resolution stays // anchored to (and confined within) the declaring stack/package root. @@ -109,8 +165,10 @@ async function loadOpenApiDocument( * generates for a hand-wired OpenAPI connector — one action per operation over a * static-auth HTTP transport, with the resolved `auth` applied. * - * Hard-fails on invalid config (missing/unfetchable spec, no base URL), so a - * misconfigured instance fails boot loudly. + * Hard-fails on invalid config (missing spec, a wrong spec URL, an unparseable + * document, a bad base URL), so a misconfigured instance fails boot loudly. A + * remote spec URL that is merely unreachable / transiently failing instead + * degrades the instance and retries (#3049) — see {@link loadOpenApiDocument}. */ export function createOpenApiProviderFactory(deps: OpenApiProviderDeps = {}): ConnectorProviderFactory { return async (ctx) => {