From cc5d6bc6325d61510e6a6176a6592965d6c8d2d7 Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Thu, 4 Jun 2026 23:46:33 -0400 Subject: [PATCH] gsc: handle stale Search Console grants during property listing (#241) --- .../gsc/SearchConsoleConnectionCard.tsx | 12 +- .../SearchConsoleOnboardingStep.tsx | 11 +- .../features/gsc/services/GscService.test.ts | 108 ++++++++++++++++-- .../features/gsc/services/GscService.ts | 46 +++++++- src/serverFunctions/gsc.ts | 7 +- 5 files changed, 168 insertions(+), 16 deletions(-) diff --git a/src/client/features/gsc/SearchConsoleConnectionCard.tsx b/src/client/features/gsc/SearchConsoleConnectionCard.tsx index 7fb915c..8d5e650 100644 --- a/src/client/features/gsc/SearchConsoleConnectionCard.tsx +++ b/src/client/features/gsc/SearchConsoleConnectionCard.tsx @@ -43,6 +43,16 @@ export function SearchConsoleConnectionCard({ queryFn: () => listGscSites({ data: { projectId } }), enabled: Boolean(showPicker && !selfHostedNeedsSetup), }); + const requiresReconnect = Boolean(sitesQuery.data?.requiresReconnect); + + React.useEffect(() => { + if (!requiresReconnect) return; + + void queryClient.invalidateQueries({ + queryKey: ["gscConnection", projectId], + }); + void queryClient.invalidateQueries({ queryKey: GRANT_STATUS_KEY }); + }, [requiresReconnect, queryClient, projectId]); const setSiteMutation = useMutation({ mutationFn: (siteUrl: string) => @@ -105,7 +115,7 @@ export function SearchConsoleConnectionCard({ ) : showPicker ? ( listGscSites({ data: { projectId } }), enabled: hasGrant && !connected && !needsSetup, }); + const requiresReconnect = Boolean(sitesQuery.data?.requiresReconnect); + + React.useEffect(() => { + if (!requiresReconnect) return; + + void queryClient.invalidateQueries({ + queryKey: ["gscConnection", projectId], + }); + }, [requiresReconnect, queryClient, projectId]); const setSiteMutation = useMutation({ mutationFn: (siteUrl: string) => @@ -107,7 +116,7 @@ function GscConnect({ projectId }: { projectId: string }) { return ( ({ - listSites: vi.fn(), - upsert: vi.fn(), - getByProjectId: vi.fn(), - deleteByProjectId: vi.fn(), - existsForConnector: vi.fn(), - dbDelete: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })), -})); +const mocks = vi.hoisted(() => { + class GscApiError extends Error { + constructor( + public readonly status: number, + message: string, + ) { + super(message); + this.name = "GscApiError"; + } + } + + class GscTokenError extends Error { + constructor(message = "token unavailable") { + super(message); + this.name = "GscTokenError"; + } + } + + return { + listSites: vi.fn(), + upsert: vi.fn(), + getByProjectId: vi.fn(), + deleteByProjectId: vi.fn(), + existsForConnector: vi.fn(), + dbDelete: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })), + GscApiError, + GscTokenError, + }; +}); vi.mock("cloudflare:workers", () => ({ env: {} })); vi.mock("@/db", () => ({ db: { delete: mocks.dbDelete } })); vi.mock("@/server/lib/gscClient", () => ({ createGscClient: () => ({ listSites: mocks.listSites }), - GscApiError: class extends Error {}, - GscTokenError: class extends Error {}, + GscApiError: mocks.GscApiError, + GscTokenError: mocks.GscTokenError, })); vi.mock("@/server/features/gsc/repositories/GscConnectionRepository", () => ({ GscConnectionRepository: { @@ -82,6 +103,73 @@ describe("GscService.setSite", () => { }); }); +describe("GscService.listSitesForUserWithGrantStatus", () => { + beforeEach(() => { + mocks.listSites.mockReset(); + mocks.dbDelete.mockClear(); + }); + + it("returns available sites when the grant is healthy", async () => { + mocks.listSites.mockResolvedValue([ + { siteUrl: "https://x/", permissionLevel: "siteOwner" }, + ]); + const { GscService } = await import("./GscService"); + + await expect( + GscService.listSitesForUserWithGrantStatus("u1"), + ).resolves.toEqual({ + sites: [{ siteUrl: "https://x/", permissionLevel: "siteOwner" }], + requiresReconnect: false, + }); + expect(mocks.dbDelete).not.toHaveBeenCalled(); + }); + + it("unlinks the dead grant and asks for reconnect when no token can be minted", async () => { + mocks.listSites.mockRejectedValue(new mocks.GscTokenError()); + const { GscService } = await import("./GscService"); + + await expect( + GscService.listSitesForUserWithGrantStatus("u1"), + ).resolves.toEqual({ sites: [], requiresReconnect: true }); + expect(mocks.dbDelete).toHaveBeenCalled(); + }); + + it("asks for reconnect without unlinking on a 403 (revoked vs. quota is ambiguous)", async () => { + mocks.listSites.mockRejectedValue( + new mocks.GscApiError(403, "Search Console denied access"), + ); + const { GscService } = await import("./GscService"); + + await expect( + GscService.listSitesForUserWithGrantStatus("u1"), + ).resolves.toEqual({ sites: [], requiresReconnect: true }); + expect(mocks.dbDelete).not.toHaveBeenCalled(); + }); + + it("asks for reconnect without unlinking on a 401", async () => { + mocks.listSites.mockRejectedValue( + new mocks.GscApiError(401, "unauthenticated"), + ); + const { GscService } = await import("./GscService"); + + await expect( + GscService.listSitesForUserWithGrantStatus("u1"), + ).resolves.toEqual({ sites: [], requiresReconnect: true }); + expect(mocks.dbDelete).not.toHaveBeenCalled(); + }); + + it("keeps non-auth GSC API errors reportable", async () => { + const rateLimit = new mocks.GscApiError(429, "slow down"); + mocks.listSites.mockRejectedValue(rateLimit); + const { GscService } = await import("./GscService"); + + await expect(GscService.listSitesForUserWithGrantStatus("u1")).rejects.toBe( + rateLimit, + ); + expect(mocks.dbDelete).not.toHaveBeenCalled(); + }); +}); + describe("GscService.disconnect", () => { beforeEach(() => { mocks.getByProjectId.mockReset(); diff --git a/src/server/features/gsc/services/GscService.ts b/src/server/features/gsc/services/GscService.ts index a521233..082057a 100644 --- a/src/server/features/gsc/services/GscService.ts +++ b/src/server/features/gsc/services/GscService.ts @@ -5,6 +5,7 @@ import { GSC_OAUTH_PROVIDER_ID } from "@/shared/gsc"; import { AppError } from "@/server/lib/errors"; import { createGscClient, + GscApiError, GscTokenError, type GscSite, type UrlInspectionResult, @@ -31,6 +32,11 @@ type GscPerformanceResult = { rows: GscSearchAnalyticsRow[]; }; +type GscSiteListResult = { + sites: GscSite[]; + requiresReconnect: boolean; +}; + /** Thrown when a project has no connected GSC property. */ export class GscNotConnectedError extends Error { constructor(public readonly projectId: string) { @@ -64,6 +70,44 @@ async function listSitesForUser(userId: string): Promise { return createGscClient({ userId }).listSites(); } +/** Expected ways a stored grant fails to reach Search Console: no token could be + * minted (refresh token revoked or expired), or Google rejected the call + * (401/403). These surface a reconnect prompt instead of being routed through + * error tracking. Other statuses (429, 5xx) are genuine faults and propagate. */ +function isExpectedGrantFailure(error: unknown): boolean { + if (error instanceof GscTokenError) return true; + return ( + error instanceof GscApiError && + (error.status === 401 || error.status === 403) + ); +} + +/** List properties for the picker UI. When the stored grant can't currently + * reach GSC, return a reconnect signal instead of throwing, so an expected + * external-auth failure doesn't land in error tracking. + * + * Only a GscTokenError unlinks the stored grant — the one unambiguous "this + * grant is dead" signal (Better Auth couldn't mint/refresh a token, i.e. the + * user revoked access or the refresh token expired). A bare 401/403 from + * sites.list is left in place: Search Console also returns 403 for quota/rate + * limits, so destroying the grant there would force needless reconnects across + * every project on it. Reconnecting re-upserts the grant either way. */ +async function listSitesForUserWithGrantStatus( + userId: string, +): Promise { + try { + return { sites: await listSitesForUser(userId), requiresReconnect: false }; + } catch (error) { + if (!isExpectedGrantFailure(error)) { + throw error; + } + if (error instanceof GscTokenError) { + await unlinkUserGrant(userId); + } + return { sites: [], requiresReconnect: true }; + } +} + /** Map a verified property to a project. Rejects unverified properties and * properties not present on the connector's grant. */ async function setSite(input: { @@ -210,7 +254,7 @@ async function inspectUrls(input: { export const GscService = { getConnection, userHasGrant, - listSitesForUser, + listSitesForUserWithGrantStatus, setSite, disconnect, getPerformance, diff --git a/src/serverFunctions/gsc.ts b/src/serverFunctions/gsc.ts index 9ba7f7d..c37ab46 100644 --- a/src/serverFunctions/gsc.ts +++ b/src/serverFunctions/gsc.ts @@ -55,12 +55,13 @@ export const listGscSites = createServerFn({ method: "POST" }) .middleware(requireProjectContext) .inputValidator((data: unknown) => projectScopedSchema.parse(data)) .handler(async ({ context }) => { - const [sites, connection] = await Promise.all([ - GscService.listSitesForUser(context.userId), + const [siteList, connection] = await Promise.all([ + GscService.listSitesForUserWithGrantStatus(context.userId), GscService.getConnection(context.projectId), ]); return { - sites: sites.map((s) => ({ + requiresReconnect: siteList.requiresReconnect, + sites: siteList.sites.map((s) => ({ siteUrl: s.siteUrl, permissionLevel: s.permissionLevel, selectable: s.permissionLevel !== "siteUnverifiedUser",