From 7036ab5b53d1213fceb142d60e7dd506033d96e8 Mon Sep 17 00:00:00 2001 From: James George <25279263+jamesgeorge007@users.noreply.github.com> Date: Wed, 29 Apr 2026 17:11:33 +0530 Subject: [PATCH] fix(common): preserve string contract for GQL history responses (#6244) --- .../hoppscotch-common/src/newstore/history.ts | 12 +++ .../persistence/__tests__/index.spec.ts | 68 ++++++++++++++ .../src/services/persistence/index.ts | 90 ++++++++++++++++++- .../src/platform/history/desktop/index.ts | 5 +- .../src/platform/history/web/index.ts | 5 +- 5 files changed, 172 insertions(+), 8 deletions(-) diff --git a/packages/hoppscotch-common/src/newstore/history.ts b/packages/hoppscotch-common/src/newstore/history.ts index 8b3dc390..3f3fcfc7 100644 --- a/packages/hoppscotch-common/src/newstore/history.ts +++ b/packages/hoppscotch-common/src/newstore/history.ts @@ -106,6 +106,18 @@ export function translateToNewGQLHistory(x: any): GQLHistoryEntry { return obj } +// Decodes a JSON-stringified wire value back to a string. Returns the +// raw value if parsing yields a non-string (object, array, etc.) so +// callers always receive a string per the GQLHistoryEntry contract. +export const decodeGQLHistoryResponse = (value: string): string => { + try { + const parsed = JSON.parse(value) + return typeof parsed === "string" ? parsed : value + } catch { + return value + } +} + export const defaultRESTHistoryState = { state: [] as RESTHistoryEntry[], } diff --git a/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts b/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts index 0f651f03..f69f5af0 100644 --- a/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts +++ b/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts @@ -709,6 +709,74 @@ describe("PersistenceService", () => { ) }) + it(`v=2 migration repairs entries in "${graphqlHistoryKey}" whose response field is a non-string and writes a pre-v2 backup`, async () => { + // Pre-fix sync writes round-tripped via JSON.stringify/parse, + // leaving entries with object-shaped response in localStorage. + const corruptedEntries = [ + { ...GQL_HISTORY_MOCK[0], response: {} }, + { ...GQL_HISTORY_MOCK[0], response: null }, + ] + await setStoreItem(graphqlHistoryKey, corruptedEntries) + + const setItemSpy = spyOnSetItem() + + await invokeSetupLocalPersistence() + + // Original is preserved at the pre-v2 backup key for recovery. + expect(setItemSpy).toHaveBeenCalledWith( + `${graphqlHistoryKey}-pre-v2-backup`, + expect.stringContaining(JSON.stringify(corruptedEntries)) + ) + + // Repaired data is written back to the live key with response coerced. + // - Object response {} stringifies to "{}", which appears in the + // serialized payload as `"response":"{}"`. + // - Null response stringifies to "null", which appears in the + // serialized payload as `"response":"null"` and preserves the + // original semantic of an empty payload. + expect(setItemSpy).toHaveBeenCalledWith( + graphqlHistoryKey, + expect.stringContaining('"response":"{}"') + ) + expect(setItemSpy).toHaveBeenCalledWith( + graphqlHistoryKey, + expect.stringContaining('"response":"null"') + ) + + // Schema version bumps to 2, so the migration won't run again. + expect(setItemSpy).toHaveBeenCalledWith( + schemaVersionKey, + expect.stringMatching(/"data":"2"/) + ) + + // No Zod-failure backup since the migration repaired the shape + // before validation could reject it. + expect(toastErrorFn).not.toHaveBeenCalledWith( + expect.stringContaining(graphqlHistoryKey) + ) + }) + + it(`v=2 migration is a no-op when "${graphqlHistoryKey}" entries already have string responses`, async () => { + // Clean entries — response is already a string per the contract. + await setStoreItem(graphqlHistoryKey, GQL_HISTORY_MOCK) + + const setItemSpy = spyOnSetItem() + + await invokeSetupLocalPersistence() + + // No backup write since needsRepair was false. + expect(setItemSpy).not.toHaveBeenCalledWith( + `${graphqlHistoryKey}-pre-v2-backup`, + expect.anything() + ) + + // Schema version still bumps to 2 so the migration is recorded as run. + expect(setItemSpy).toHaveBeenCalledWith( + schemaVersionKey, + expect.stringMatching(/"data":"2"/) + ) + }) + it(`GQL history schema parsing succeeds if there is no "${graphqlHistoryKey}" key present in localStorage where the fallback of "[]" is chosen`, async () => { window.localStorage.removeItem(graphqlHistoryKey) diff --git a/packages/hoppscotch-common/src/services/persistence/index.ts b/packages/hoppscotch-common/src/services/persistence/index.ts index 83d9e823..e722cb25 100644 --- a/packages/hoppscotch-common/src/services/persistence/index.ts +++ b/packages/hoppscotch-common/src/services/persistence/index.ts @@ -176,6 +176,76 @@ const migrations: Migration[] = [ } }, }, + { + // Coerce gqlHistory entries with non-string `response` to string, + // backing up originals at `${GQL_HISTORY}-pre-v2-backup`. + version: 2, + migrate: async () => { + const result = await Store.get( + STORE_NAMESPACE, + STORE_KEYS.GQL_HISTORY + ) + + if (!E.isRight(result) || !Array.isArray(result.right)) return + + const entries = result.right + // Only target entries with own `response` field that's non-string; + // unrelated schema mismatches still go through the Zod-fail backup. + const needsRepair = entries.some( + (entry) => + typeof entry === "object" && + entry !== null && + Object.prototype.hasOwnProperty.call(entry, "response") && + typeof (entry as { response?: unknown }).response !== "string" + ) + + if (!needsRepair) return + + // Throw on backup or repair write failure so `runMigrations` can + // skip the schema_version bump and retry on the next launch. The + // alternative — log-and-continue — would mark the migration done + // while leaving poisoned data in place, with no future retry path. + const backupResult = await Store.set( + STORE_NAMESPACE, + `${STORE_KEYS.GQL_HISTORY}-pre-v2-backup`, + entries + ) + if (E.isLeft(backupResult)) { + throw new Error( + `[v2 migration] failed to write pre-v2 backup: ${backupResult.left.kind}: ${backupResult.left.message}` + ) + } + + const repaired = entries.map((entry) => { + if (typeof entry !== "object" || entry === null) return entry + const e = entry as Record + if ( + !Object.prototype.hasOwnProperty.call(e, "response") || + typeof e.response === "string" + ) { + return e + } + // Use JSON.stringify(e.response) directly so a `null` payload + // serializes to the string `"null"` rather than `'""'`. Either + // form satisfies the `response: z.string()` schema, but `"null"` + // preserves the original semantic of an empty payload (e.g. a + // subscription that produced no data) and avoids re-stringifying + // on the next sync write. + return { ...e, response: JSON.stringify(e.response) } + }) + + const repairResult = await Store.set( + STORE_NAMESPACE, + STORE_KEYS.GQL_HISTORY, + repaired + ) + if (E.isLeft(repairResult)) { + throw new Error( + `[v2 migration] failed to write repaired ${STORE_KEYS.GQL_HISTORY}: ${repairResult.left.kind}: ${repairResult.left.message}` + ) + } + }, + }, ] /** @@ -235,13 +305,25 @@ export class PersistenceService extends Service { ) const perhapsVersion = E.isRight(versionResult) ? versionResult.right : "0" const currentVersion = perhapsVersion ?? "0" - const targetVersion = "1" + const targetVersion = "2" if (currentVersion !== targetVersion) { - for (const migration of migrations) { - if (migration.version > parseInt(currentVersion)) { - await migration.migrate() + try { + for (const migration of migrations) { + if (migration.version > parseInt(currentVersion)) { + await migration.migrate() + } } + } catch (err) { + // A migration that throws (e.g. v2 repair on a degraded store) + // aborts the schema_version bump so the next launch retries + // from the same currentVersion rather than recording an + // incomplete migration as done. + console.error( + "[persistence] migration failed; schema_version not advanced:", + err + ) + return } await Store.set(STORE_NAMESPACE, STORE_KEYS.SCHEMA_VERSION, targetVersion) diff --git a/packages/hoppscotch-selfhost-web/src/platform/history/desktop/index.ts b/packages/hoppscotch-selfhost-web/src/platform/history/desktop/index.ts index f36240d5..dccf9e93 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/history/desktop/index.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/history/desktop/index.ts @@ -14,6 +14,7 @@ import { graphqlHistoryStore, deleteGraphqlHistoryEntry, clearGraphqlHistory, + decodeGQLHistoryResponse, } from "@hoppscotch/common/newstore/history" import { translateToNewRequest, translateToGQLRequest } from "@hoppscotch/data" import { HistoryPlatformDef } from "@hoppscotch/common/platform/history" @@ -109,7 +110,7 @@ async function loadHistoryEntries() { const gqlHistoryEntries: GQLHistoryEntry[] = gqlEntries.map((entry) => ({ v: 1, request: translateToGQLRequest(JSON.parse(entry.request)), - response: JSON.parse(entry.responseMetadata), + response: decodeGQLHistoryResponse(entry.responseMetadata), star: entry.isStarred, updatedOn: new Date(entry.executedOn), id: entry.id, @@ -175,7 +176,7 @@ function setupUserHistoryCreatedSubscription() { v: 1, id, request: translateToGQLRequest(JSON.parse(request)), - response: JSON.parse(responseMetadata), + response: decodeGQLHistoryResponse(responseMetadata), star: isStarred, updatedOn: new Date(executedOn), }) diff --git a/packages/hoppscotch-selfhost-web/src/platform/history/web/index.ts b/packages/hoppscotch-selfhost-web/src/platform/history/web/index.ts index 64e1186c..9a274161 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/history/web/index.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/history/web/index.ts @@ -14,6 +14,7 @@ import { graphqlHistoryStore, deleteGraphqlHistoryEntry, clearGraphqlHistory, + decodeGQLHistoryResponse, } from "@hoppscotch/common/newstore/history" import { translateToNewRequest, translateToGQLRequest } from "@hoppscotch/data" import { HistoryPlatformDef } from "@hoppscotch/common/platform/history" @@ -112,7 +113,7 @@ async function loadHistoryEntries() { const gqlHistoryEntries: GQLHistoryEntry[] = gqlEntries.map((entry) => ({ v: 1, request: translateToGQLRequest(JSON.parse(entry.request)), - response: JSON.parse(entry.responseMetadata), + response: decodeGQLHistoryResponse(entry.responseMetadata), star: entry.isStarred, updatedOn: new Date(entry.executedOn), id: entry.id, @@ -178,7 +179,7 @@ function setupUserHistoryCreatedSubscription() { v: 1, id, request: translateToGQLRequest(JSON.parse(request)), - response: JSON.parse(responseMetadata), + response: decodeGQLHistoryResponse(responseMetadata), star: isStarred, updatedOn: new Date(executedOn), })