fix(common): preserve string contract for GQL history responses (#6244)
This commit is contained in:
James George
GitHub
parent
aee017ced3
commit
7036ab5b53
5 changed files with 172 additions and 8 deletions
|
|
@ -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[],
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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<unknown>(
|
||||
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<string, unknown>
|
||||
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,14 +305,26 @@ 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) {
|
||||
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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue