diff --git a/CHANGELOG.md b/CHANGELOG.md index d446e043..92176e9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,26 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Connecting an account survives a vendor that is down + +Finishing a connection used to end on a blank server error if the vendor could not be reached at the +moment you were sent back, whether that was a refused connection, a name that would not resolve, or +fifteen seconds of silence. It now ends where every other failed connect already ended: back on +Connected accounts with a note, and nothing stored. Pressing Connect for a vendor this deployment +has not introduced itself to yet behaves the same way, answering 502 rather than a server error, and +that message now covers a vendor that could not be reached as well as one that turned the +registration down. + +A connection whose grant cannot be written to the vault ends the same way, rather than on the blank +error it used to give somebody who had just finished consenting. + +Because the person is told the same thing whatever went wrong, the server log is now where the +difference lives. Three lines to look for: `oauth-token-endpoint-unreachable` and +`oauth-registration-endpoint-unreachable` name the vendor and the cause, and +`oauth-connection-not-recorded` says a consent completed and could not be kept. A fourth, +`oauth-token-endpoint-unusable`, means the fault is this deployment's catalogue rather than the +vendor. + ### Coworkers are made in a wizard and managed in a dialog Creating a coworker is now a three-step wizard — who it is, who may see it, then where it runs, diff --git a/server/src/plugins/oauth.ts b/server/src/plugins/oauth.ts index 5d9c2b70..9ce11833 100644 --- a/server/src/plugins/oauth.ts +++ b/server/src/plugins/oauth.ts @@ -323,20 +323,81 @@ export async function redeemAuthorizationCode(input: { // A public (DCR) client proves itself with PKCE, and some vendors refuse an unexpected empty field. if (input.clientSecret) params.set("client_secret", input.clientSecret); - const response = await fetch(input.tokenUrl, { - method: "POST", - headers: { "content-type": "application/x-www-form-urlencoded" }, - body: params, + /* + * Our own catalogue, told apart from their outage before a request is attempted. + * + * `fetch` refuses a malformed URL by throwing the same kind of error a refused connection does, + * so without this the two would reach the same catch and read as the same sentence. They are not + * the same thing: one is a vendor having a bad day and the other is an endpoint in this + * deployment's own catalogue that nobody can ever connect through. The person gets the ordinary + * refusal either way, because there is nothing else to give them, and the log is where the + * difference has to survive. + * + * Checking it here is also what leaves the catch below covering only the transport, rather than + * quietly standing in for a mistake in a frozen literal. + */ + if (!URL.canParse(input.tokenUrl)) { + console.error( + JSON.stringify({ + type: "oauth-token-endpoint-unusable", + tokenUrl: input.tokenUrl, + note: "This vendor's token endpoint is not a usable address, so nobody can connect it until the catalogue is fixed.", + }), + ); + return null; + } + + /* + * A vendor that could not be reached at all, which the refusal below cannot see. + * + * `!response.ok` needs a response, and there is none when the connection is refused, the name does + * not resolve, TLS will not agree, or the timeout on this request fires. Unguarded, that rejection + * escaped a function whose whole contract is to refuse quietly, and it escaped at the worst + * available moment: the callback has no failure handler above it, so somebody who had just + * consented at the vendor got a bare 500 with no Location instead of Settings telling them it did + * not work. The same reasoning as the defensive read further down, one step earlier in the request. + * + * Nothing is read off the error. Which of those it was is a fact about the vendor's infrastructure + * on a route that answers an unauthenticated caller, and the person is told the same sentence + * either way. + */ + let response: Response; + try { + response = await fetch(input.tokenUrl, { + method: "POST", + headers: { "content-type": "application/x-www-form-urlencoded" }, + body: params, + /* + * A redirect is a refusal, not a detour to be followed. + * + * `tokenUrl` is pinned in the catalogue because this request carries a client secret and an + * authorization code, and following a 302 would hand both to whatever address the answer named. + * Manual leaves the 3xx as the response, which is not `ok`, so it falls into the refusal below. + */ + redirect: "manual", + signal: AbortSignal.timeout(15_000), + }); + } catch (error) { /* - * A redirect is a refusal, not a detour to be followed. + * Logged, because refusing quietly to the person must not mean refusing quietly to the + * deployment. Until this, an unreachable token endpoint reached Hono's default handler, which + * prints the error before its 500; catching it without a line here would have bought the + * redirect by making a vendor outage look exactly like nobody trying to connect. * - * `tokenUrl` is pinned in the catalogue because this request carries a client secret and an - * authorization code, and following a 302 would hand both to whatever address the answer named. - * Manual leaves the 3xx as the response, which is not `ok`, so it falls into the refusal below. + * The status is what a person sees and this is what an operator sees, and only the second one + * says which vendor and why. Neither the code nor the client secret is in a transport error: + * they are in the request body, which never got sent. */ - redirect: "manual", - signal: AbortSignal.timeout(15_000), - }); + console.error( + JSON.stringify({ + type: "oauth-token-endpoint-unreachable", + tokenUrl: input.tokenUrl, + note: "A person's consent could not be redeemed. They were sent back to Settings with a failure.", + error: String(error), + }), + ); + return null; + } if (!response.ok) return null; @@ -388,21 +449,55 @@ export async function registerDynamicClient(input: { registrationUrl: string; redirectUri: string; }): Promise<{ clientId: string; clientSecret: string } | null> { - const response = await fetch(input.registrationUrl, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - redirect_uris: [input.redirectUri], - grant_types: ["authorization_code", "refresh_token"], - response_types: ["code"], - token_endpoint_auth_method: "none", - client_name: "OpenBot", - }), - // The registration endpoint is pinned in the catalogue, so a redirect is somebody else deciding - // where this deployment introduces itself. Left as the response, which is not `ok`. - redirect: "manual", - signal: AbortSignal.timeout(15_000), - }); + // The same catalogue check the redemption makes, and for the same reason: a registration endpoint + // that is not an address is this deployment's mistake, not the vendor's outage, and only the log + // can tell them apart. + if (!URL.canParse(input.registrationUrl)) { + console.error( + JSON.stringify({ + type: "oauth-registration-endpoint-unusable", + registrationUrl: input.registrationUrl, + note: "This vendor's registration endpoint is not a usable address, so this deployment can never introduce itself.", + }), + ); + return null; + } + + // A vendor that could not be reached at all — see `redeemAuthorizationCode`, which states the same + // gap in full. This one lands on an administrator pressing Connect rather than on somebody + // mid-consent, and null is what turns it into the 502 that route already writes for a vendor that + // would not register us. + let response: Response; + try { + response = await fetch(input.registrationUrl, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + redirect_uris: [input.redirectUri], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + client_name: "OpenBot", + }), + // The registration endpoint is pinned in the catalogue, so a redirect is somebody else deciding + // where this deployment introduces itself. Left as the response, which is not `ok`. + redirect: "manual", + signal: AbortSignal.timeout(15_000), + }); + } catch (error) { + // Logged for the same reason the redemption above is: the caller's 502 tells an administrator + // to check the vendor's status, and this is the line that says the vendor could not be reached + // at all rather than that it turned us down. + console.error( + JSON.stringify({ + type: "oauth-registration-endpoint-unreachable", + registrationUrl: input.registrationUrl, + note: "This deployment could not introduce itself to the vendor. Connect answered 502.", + error: String(error), + }), + ); + return null; + } if (!response.ok) return null; // A 200 is not a promise of JSON — see `redeemAuthorizationCode`. A body that will not parse is // the vendor answering with something other than a client, which is this function's null. diff --git a/server/src/plugins/routes.ts b/server/src/plugins/routes.ts index d03ac556..8e9e22c8 100644 --- a/server/src/plugins/routes.ts +++ b/server/src/plugins/routes.ts @@ -415,7 +415,7 @@ export function createPluginRoutes( if (entry.auth.clientRegistration === "dynamic") { return context.json( { - error: `${entry.title} refused this deployment's registration. Try again, and check the vendor's status if it persists.`, + error: `${entry.title} would not register this deployment, or could not be reached. Try again, and check the vendor's status if it persists.`, }, 502, ); @@ -514,12 +514,36 @@ export function createPluginRoutes( }); if (!grant) return context.redirect(failed); - await store.recordConnection({ - serverId: state.serverId, - userId: state.userId, - refreshToken: grant.refreshToken, - scope: grant.scope, - }); + /* + * The last thing that can fail, answered the same way as everything before it. + * + * A vault that will not take the grant is this deployment's problem, not the person's, and they + * have already done their part at the vendor. Unhandled, this threw past the handler and gave + * them the bare 500 that every other failure on this route was written to avoid, on the one + * path where they had most reason to think it had worked. + * + * Told, because unlike the refusals above this one is nobody's fault but ours, and the person's + * sentence deliberately says nothing about which failure it was. The refresh token is not + * logged: it is the one thing here worth stealing, and the row it belonged to was never written. + */ + try { + await store.recordConnection({ + serverId: state.serverId, + userId: state.userId, + refreshToken: grant.refreshToken, + scope: grant.scope, + }); + } catch (error) { + console.error( + JSON.stringify({ + type: "oauth-connection-not-recorded", + serverId: state.serverId, + note: "A person consented and the grant could not be stored. They were sent back to Settings with a failure and will have to connect again.", + error: String(error), + }), + ); + return context.redirect(failed); + } return context.redirect( connectedAccountsUrlFor( diff --git a/server/tests/plugin-connect-route.test.ts b/server/tests/plugin-connect-route.test.ts index ccdcf46b..3aa9617f 100644 --- a/server/tests/plugin-connect-route.test.ts +++ b/server/tests/plugin-connect-route.test.ts @@ -82,7 +82,15 @@ describe("connecting a dynamically registered vendor", () => { expect(url.searchParams.get("client_id")).toBe("dyn-1"); }); - test("a refused registration answers 502, naming the vendor", async () => { + /** + * One answer for two states, because there is one thing to do about either. + * + * A vendor that turned this deployment down and a vendor that could not be reached both leave + * `ensureOAuthClient` with no client, and the person pressing Connect has the same next step + * whichever it was. The sentence says both rather than naming the wrong one confidently; which it + * actually was is in the log, where it is of use to somebody who can act on it. + */ + test("a registration that produced no client answers 502, naming the vendor", async () => { const ensureCalls: { serverId: string; by: string }[] = []; const hono = app({ oauthClientFor: async () => null, @@ -101,7 +109,7 @@ describe("connecting a dynamically registered vendor", () => { expect(ensureCalls.length).toBe(1); const body = (await response.json()) as { error: string }; expect(body.error).toBe( - "Notion refused this deployment's registration. Try again, and check the vendor's status if it persists.", + "Notion would not register this deployment, or could not be reached. Try again, and check the vendor's status if it persists.", ); }); }); diff --git a/server/tests/plugin-oauth-callback.test.ts b/server/tests/plugin-oauth-callback.test.ts index 757d84a7..a3d3f5c8 100644 --- a/server/tests/plugin-oauth-callback.test.ts +++ b/server/tests/plugin-oauth-callback.test.ts @@ -2,7 +2,11 @@ import { describe, expect, test } from "bun:test"; import type { MiddlewareHandler } from "hono"; import { Hono } from "hono"; import type { AppVariables } from "../src/auth/guards"; -import { challengeFor, sealConnectState } from "../src/plugins/oauth"; +import { + challengeFor, + redeemAuthorizationCode, + sealConnectState, +} from "../src/plugins/oauth"; import { createPluginRoutes } from "../src/plugins/routes"; /** @@ -48,13 +52,17 @@ function app(input: { recorded: Recorded[]; /** Whether the person named by the state still has access. Present by default. */ personHasAccess?: (userId: string) => Promise; + /** What the vault does with the grant. Records it by default; a test may refuse instead. */ + recordConnection?: (connection: Recorded) => Promise; }) { const store = { oauthClientFor: async () => ({ clientId: "dyn-1", clientSecret: "" }), ensureOAuthClient: async () => ({ clientId: "dyn-1", clientSecret: "" }), - recordConnection: async (connection: Recorded) => { - input.recorded.push(connection); - }, + recordConnection: + input.recordConnection ?? + (async (connection: Recorded) => { + input.recorded.push(connection); + }), }; const routes = createPluginRoutes( store as never, @@ -229,3 +237,225 @@ describe("a consent this deployment did not start", () => { expect(recorded).toEqual([]); }); }); + +/** + * The vendor answered the consent screen and then could not be reached for the redemption. + * + * Every other refusal here is one of our own checks saying no before the network is touched, which + * is why nothing caught this: the vendor in these tests is either willing or never asked. A vendor + * that is reachable enough to send somebody back and unreachable a moment later is the ordinary + * shape of an outage, and it lands on somebody who has just consented. + */ +describe("a vendor that could not be reached for the redemption", () => { + async function sealed(): Promise { + return await sealConnectState( + { userId: "user-1", serverId: "notion", verifier: "v-1" }, + KEY, + ); + } + + /** + * The signal is inspected rather than ignored, which is the difference between proving the catch + * runs and proving it runs on the failure it was written for. A real timeout rejects because the + * request was already on the wire when `AbortSignal.timeout` fired, so a stub that never looked at + * `init.signal` would pass just as happily against code that had forgotten to pass one. + * + * What is still not exercised is the fifteen seconds themselves: the deadline is a literal in the + * source, and waiting it out is not a test anybody would run. The signal being present and unfired + * at the moment of the call is the part that can be checked here. + */ + async function withUnreachableVendor( + reject: () => never, + run: () => Promise, + seen?: { signals: (AbortSignal | undefined)[] }, + ): Promise { + const realFetch = globalThis.fetch; + globalThis.fetch = (async (_url: unknown, init?: RequestInit) => { + seen?.signals.push(init?.signal ?? undefined); + return reject(); + }) as unknown as typeof fetch; + try { + await run(); + } finally { + globalThis.fetch = realFetch; + } + } + + test("a connection failure ends at Settings, not on a 500", async () => { + const recorded: Recorded[] = []; + const hono = app({ recorded }); + const state = await sealed(); + + await withUnreachableVendor( + () => { + throw new TypeError( + "Unable to connect. Is the computer able to access the url?", + ); + }, + async () => { + const response = await hono.request(callbackUrl(state)); + // Both halves of the promise the handler makes, and the status is the half that was broken: + // a throw out of the redemption left Hono answering 500 with no Location at all. + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe(FAILED); + }, + ); + expect(recorded).toEqual([]); + }); + + test("a token endpoint that never answers ends the same way", async () => { + const recorded: Recorded[] = []; + const hono = app({ recorded }); + const state = await sealed(); + const seen = { signals: [] as (AbortSignal | undefined)[] }; + + await withUnreachableVendor( + () => { + throw new DOMException("The operation timed out.", "TimeoutError"); + }, + async () => { + const response = await hono.request(callbackUrl(state)); + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe(FAILED); + }, + seen, + ); + + // The deadline was armed and had not fired when the request went out, which is what makes this + // a timeout rather than a rejection that happened to arrive first. + expect(seen.signals[0]).toBeInstanceOf(AbortSignal); + expect(seen.signals[0]?.aborted).toBe(false); + expect(recorded).toEqual([]); + }); + + /** + * Quiet to the person, not quiet to the deployment. + * + * The redirect above is deliberately the same one every other refusal produces, which is what + * makes this test necessary: from the outside a vendor that is down and a vendor that said no are + * now indistinguishable, so the only place the difference survives is the log. Before the refusal + * was caught at all, the framework's own handler printed it on the way to a 500; catching it + * without putting a line back would have paid for the redirect with the outage nobody can see. + */ + test("the deployment is told, even though the person is only sent back", async () => { + const recorded: Recorded[] = []; + const hono = app({ recorded }); + const state = await sealed(); + const said: string[] = []; + const realError = console.error; + console.error = (...args: unknown[]) => { + said.push(args.map(String).join(" ")); + }; + + try { + await withUnreachableVendor( + () => { + throw new TypeError("Unable to connect."); + }, + async () => { + const response = await hono.request(callbackUrl(state)); + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe(FAILED); + }, + ); + } finally { + console.error = realError; + } + + const line = said.find((said) => + said.includes("oauth-token-endpoint-unreachable"), + ); + expect(line).toBeDefined(); + // Which vendor, so an operator reading this knows where to look, and the cause. + expect(line).toContain("mcp.notion.com"); + expect(line).toContain("Unable to connect."); + expect(recorded).toEqual([]); + }); +}); + +/** + * The last thing that can fail, and the one the redemption fix did not reach. + * + * Everything before this point answers a failure with the same redirect. Writing the grant did not: + * a vault that will not take it threw past the handler, and the person who had just consented got + * the same bare 500 that an unreachable vendor used to give them. It is the identical shape one step + * later, so it gets the identical answer. + */ +describe("a grant the vault would not take", () => { + test("a refused write ends at Settings, and no half-connection is claimed", async () => { + const recorded: Recorded[] = []; + const hono = app({ + recorded, + recordConnection: async () => { + throw new Error("could not reach the database"); + }, + }); + const state = await sealConnectState( + { userId: "user-1", serverId: "notion", verifier: "v-1" }, + KEY, + ); + const said: string[] = []; + const realError = console.error; + console.error = (...args: unknown[]) => { + said.push(args.map(String).join(" ")); + }; + + try { + await withWillingVendor(async () => { + const response = await hono.request(callbackUrl(state)); + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe(FAILED); + }); + } finally { + console.error = realError; + } + + expect(recorded).toEqual([]); + // Told, for the same reason the unreachable vendor is: this one is the deployment's own fault, + // and the person is being handed the sentence that says nothing about which. + expect( + said.find((line) => line.includes("oauth-connection-not-recorded")), + ).toBeDefined(); + }); +}); + +/** + * A pinned endpoint that is not an address. + * + * `fetch` refuses a malformed URL by throwing the same kind of error a refused connection does, so + * the catch that made the vendor's outage quiet would make this quiet in exactly the same words. + * The person still gets the ordinary refusal, because there is nothing else to give them, but the + * log has to say which of the two it was: one is somebody else's outage and the other is this + * deployment's own catalogue, and only one of them is worth waking up for. + */ +describe("a token endpoint that is not a usable address", () => { + test("is refused like any other, and named as ours in the log", async () => { + const said: string[] = []; + const realError = console.error; + console.error = (...args: unknown[]) => { + said.push(args.map(String).join(" ")); + }; + + try { + expect( + await redeemAuthorizationCode({ + tokenUrl: "not a url", + clientId: "dyn-1", + clientSecret: "", + code: "code-1", + redirectUri: "https://openbot.example/api/plugins/oauth/callback", + verifier: "v-1", + }), + ).toBeNull(); + } finally { + console.error = realError; + } + + expect( + said.find((line) => line.includes("oauth-token-endpoint-unusable")), + ).toBeDefined(); + expect( + said.find((line) => line.includes("oauth-token-endpoint-unreachable")), + ).toBeUndefined(); + }); +}); diff --git a/server/tests/plugin-oauth.test.ts b/server/tests/plugin-oauth.test.ts index b8487c9d..ba13c4a0 100644 --- a/server/tests/plugin-oauth.test.ts +++ b/server/tests/plugin-oauth.test.ts @@ -558,6 +558,60 @@ describe("registering this deployment as an OAuth client", () => { globalThis.fetch = realFetch; } }); + + /** + * A vendor that cannot be reached at all, which is not the same as one that refused. + * + * `!response.ok` needs a response, and there is none when the connection is refused, the name does + * not resolve, TLS will not agree or the fifteen-second timeout fires first. Unguarded, the fetch + * rejects straight through a function whose documented answer to a vendor that will not register + * us is null, and the caller that reads null to mean "try again later" never runs. + */ + test("a vendor that cannot be reached is a refusal, not a thrown transport error", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = (async () => { + throw new TypeError( + "Unable to connect. Is the computer able to access the url?", + ); + }) as unknown as typeof fetch; + try { + expect( + await registerDynamicClient({ + registrationUrl: "https://vendor.example/register", + redirectUri: "https://openbot.example/cb", + }), + ).toBeNull(); + } finally { + globalThis.fetch = realFetch; + } + }); + + /** + * The signal is checked, not just the catch. A stub that ignores `init.signal` would pass against + * code that had dropped the deadline entirely, which is the one thing a timeout test exists to + * notice. The fifteen seconds themselves stay unexercised; the literal is in the source and + * waiting it out is not a test. + */ + test("a registration endpoint that never answers is the same refusal", async () => { + const seen: (AbortSignal | undefined)[] = []; + const realFetch = globalThis.fetch; + globalThis.fetch = (async (_url: unknown, init?: RequestInit) => { + seen.push(init?.signal ?? undefined); + throw new DOMException("The operation timed out.", "TimeoutError"); + }) as unknown as typeof fetch; + try { + expect( + await registerDynamicClient({ + registrationUrl: "https://vendor.example/register", + redirectUri: "https://openbot.example/cb", + }), + ).toBeNull(); + } finally { + globalThis.fetch = realFetch; + } + expect(seen[0]).toBeInstanceOf(AbortSignal); + expect(seen[0]?.aborted).toBe(false); + }); }); describe("redeeming an authorization code", () => { @@ -699,4 +753,62 @@ describe("redeeming an authorization code", () => { globalThis.fetch = realFetch; } }); + + /** + * A token endpoint that cannot be reached at all. + * + * The one failure with no response to inspect, so `!response.ok` never sees it: a refused + * connection, a name that does not resolve, TLS that will not agree, or the fifteen-second timeout + * firing. It is also the failure that lands at the worst moment, on somebody who has just consented + * at the vendor and is being sent back here. Unguarded it escapes the callback as a bare 500 with + * no Location, so instead of Settings saying it did not work, they get an error page. + */ + test("a token endpoint that cannot be reached is a refusal, not a thrown transport error", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = (async () => { + throw new TypeError( + "Unable to connect. Is the computer able to access the url?", + ); + }) as unknown as typeof fetch; + try { + expect( + await redeemAuthorizationCode({ + tokenUrl: "https://vendor.example/token", + clientId: "client-id", + clientSecret: "", + code: "code-1", + redirectUri: "https://openbot.example/api/plugins/oauth/callback", + verifier: "verifier-1", + }), + ).toBeNull(); + } finally { + globalThis.fetch = realFetch; + } + }); + + /** Same reasoning as the registration side: the deadline is what is being tested, so it is read. */ + test("a token endpoint that never answers is the same refusal", async () => { + const seen: (AbortSignal | undefined)[] = []; + const realFetch = globalThis.fetch; + globalThis.fetch = (async (_url: unknown, init?: RequestInit) => { + seen.push(init?.signal ?? undefined); + throw new DOMException("The operation timed out.", "TimeoutError"); + }) as unknown as typeof fetch; + try { + expect( + await redeemAuthorizationCode({ + tokenUrl: "https://vendor.example/token", + clientId: "client-id", + clientSecret: "", + code: "code-1", + redirectUri: "https://openbot.example/api/plugins/oauth/callback", + verifier: "verifier-1", + }), + ).toBeNull(); + } finally { + globalThis.fetch = realFetch; + } + expect(seen[0]).toBeInstanceOf(AbortSignal); + expect(seen[0]?.aborted).toBe(false); + }); }); diff --git a/server/tests/plugin-store.integration.test.ts b/server/tests/plugin-store.integration.test.ts index f05914ff..89e2ef83 100644 --- a/server/tests/plugin-store.integration.test.ts +++ b/server/tests/plugin-store.integration.test.ts @@ -2018,6 +2018,67 @@ describe("a dynamic client the vendor has evicted", () => { ); }); + /** + * The registration nothing stands in for. + * + * Every other test here injects `registerClient`, which is right for asserting what the store does + * with an answer but means the real function is never the one answering. What it returns when the + * vendor cannot be reached at all is exactly what the store's `null` branches were written for, so + * once that path exists it is worth one test that lets the real code produce the value rather than + * a stub asserting the value the real code is assumed to produce. + */ + const storeWithRealRegistration = createPluginStore({ + database, + auditStore: createAuditStore(database), + credentials: vault, + encryptionKey: DYNAMIC_KEY, + policy: () => policy, + callVendor: async () => ({ + text: "[vendor not reached in tests]", + isError: false, + }), + exchangeRefreshToken: seams.exchangeRefreshToken, + redirectUri: REDIRECT_URI, + }); + + test("an unreachable registration endpoint leaves no client and no trail", async () => { + await clearClient(); + const registeredBefore = await registeredRows(); + const said: string[] = []; + const realError = console.error; + const realFetch = globalThis.fetch; + console.error = (...args: unknown[]) => { + said.push(args.map(String).join(" ")); + }; + globalThis.fetch = (async () => { + throw new TypeError("Unable to connect."); + }) as unknown as typeof fetch; + + try { + expect( + await storeWithRealRegistration.ensureOAuthClient( + dynamicServerId, + "someone@openbot.test", + ), + ).toBeNull(); + } finally { + globalThis.fetch = realFetch; + console.error = realError; + } + + // Nothing kept, and nothing claimed. A trail row here would say this deployment registered + // itself with a vendor that never answered. + expect( + await storeWithRealRegistration.oauthClientFor(dynamicServerId), + ).toBe(null); + expect((await registeredRows()).length).toBe(registeredBefore.length); + expect( + said.find((line) => + line.includes("oauth-registration-endpoint-unreachable"), + ), + ).toBeDefined(); + }); + test("an entry an administrator registers by hand is left alone", async () => { /* * Drive, whose client is pasted in from Google's console. Registering one for it would be