Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: retry invalid/expired refs #356

Merged
merged 7 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 74 additions & 7 deletions src/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ export const GET_ALL_QUERY_DELAY = 500
*/
const DEFUALT_RETRY_AFTER_MS = 1000

/**
* The maximum number of attemps to retry a query with an invalid ref. We allow
* multiple attempts since each attempt may use a different (and possibly
* invalid) ref. Capping the number of attemps prevents infinite loops.
*/
const MAX_INVALID_REF_RETRY_ATTEMPS = 3

/**
* Extracts one or more Prismic document types that match a given Prismic
* document type. If no matches are found, no extraction is performed and the
Expand Down Expand Up @@ -529,9 +536,9 @@ export class Client<
async get<TDocument extends TDocuments>(
params?: Partial<BuildQueryURLArgs> & FetchParams,
): Promise<Query<TDocument>> {
const url = await this.buildQueryURL(params)
const { data } = await this._get<TDocument>(params)

return await this.fetch<Query<TDocument>>(url, params)
return data
}

/**
Expand All @@ -546,8 +553,9 @@ export class Client<
*
* @typeParam TDocument - Type of the Prismic document returned.
*
* @param params - Parameters to filter, sort, and paginate results. @returns
* The first result of the query, if any.
* @param params - Parameters to filter, sort, and paginate results.
*
* @returns The first result of the query, if any.
*/
async getFirst<TDocument extends TDocuments>(
params?: Partial<BuildQueryURLArgs> & FetchParams,
Expand All @@ -556,10 +564,9 @@ export class Client<
if (!(params && params.page) && !params?.pageSize) {
actualParams.pageSize = this.defaultParams?.pageSize ?? 1
}
const url = await this.buildQueryURL(actualParams)
const result = await this.fetch<Query<TDocument>>(url, params)
const { data, url } = await this._get<TDocument>(actualParams)

const firstResult = result.results[0]
const firstResult = data.results[0]

if (firstResult) {
return firstResult
Expand Down Expand Up @@ -1678,6 +1685,66 @@ export class Client<
return findMasterRef(cachedRepository.refs).ref
}

/**
* The private implementation of `this.get`. It returns the API response and
* the URL used to make the request. The URL is sometimes used in the public
* method to include in thrown errors.
*
* This method retries requests that throw `RefNotFoundError` or
* `RefExpiredError`. It contains special logic to retry with the latest
* master ref, provided in the API's error message.
*
* @typeParam TDocument - Type of Prismic documents returned.
*
* @param params - Parameters to filter, sort, and paginate results.
*
* @returns An object containing the paginated response containing the result
* of the query and the URL used to make the API request.
*/
private async _get<TDocument extends TDocuments>(
params?: Partial<BuildQueryURLArgs> & FetchParams,
attemptCount = 0,
): Promise<{ data: Query<TDocument>; url: string }> {
const url = await this.buildQueryURL(params)

try {
const data = await this.fetch<Query<TDocument>>(url, params)

return { data, url }
} catch (error) {
if (
!(
error instanceof RefNotFoundError || error instanceof RefExpiredError
) ||
attemptCount >= MAX_INVALID_REF_RETRY_ATTEMPS - 1
) {
throw error
}

// If no explicit ref is given (i.e. the master ref from
// /api/v2 is used), clear the cached repository value.
// Clearing the cached value prevents other methods from
// using a known-stale ref.
if (!params?.ref) {
this.cachedRepository = undefined
}

const masterRef = error.message.match(/Master ref is: (?<ref>.*)$/)
?.groups?.ref
if (!masterRef) {
throw error
}

const badRef = new URL(url).searchParams.get("ref")
const issue = error instanceof RefNotFoundError ? "invalid" : "expired"
console.warn(
`The ref (${badRef}) was ${issue}. Now retrying with the latest master ref (${masterRef}). If you were previewing content, the response will not include draft content.`,
)

return await this._get({ ...params, ref: masterRef }, attemptCount + 1)
}
}

/**
* Performs a network request using the configured `fetch` function. It
* assumes all successful responses will have a JSON content type. It also
Expand Down
170 changes: 170 additions & 0 deletions test/__testutils__/testInvalidRefRetry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import { expect, it, vi } from "vitest"

import { rest } from "msw"

import { createTestClient } from "./createClient"
import { mockPrismicRestAPIV2 } from "./mockPrismicRestAPIV2"

import * as prismic from "../../src"

type TestInvalidRefRetryArgs = {
run: (
client: prismic.Client,
params?: Parameters<prismic.Client["get"]>[0],
) => Promise<unknown>
}

export const testInvalidRefRetry = (args: TestInvalidRefRetryArgs): void => {
it("retries with the master ref when an invalid ref is used", async (ctx) => {
const client = createTestClient({ ctx })
const badRef = ctx.mock.api.ref().ref
const masterRef = ctx.mock.api.ref().ref
const queryResponse = ctx.mock.api.query({
documents: [ctx.mock.value.document()],
})

const triedRefs = new Set<string | null>()

mockPrismicRestAPIV2({ ctx, queryResponse })
const endpoint = new URL(
"documents/search",
`${client.documentAPIEndpoint}/`,
).toString()
ctx.server.use(
rest.get(endpoint, (req) => {
triedRefs.add(req.url.searchParams.get("ref"))
}),
rest.get(endpoint, (_req, res, ctx) =>
res.once(
ctx.json({
type: "api_notfound_error",
message: `Master ref is: ${masterRef}`,
}),
ctx.status(404),
),
),
)

const consoleWarnSpy = vi
.spyOn(console, "warn")
.mockImplementation(() => void 0)
await args.run(client, { ref: badRef })
consoleWarnSpy.mockRestore()

expect([...triedRefs]).toStrictEqual([badRef, masterRef])
})

it("retries with the master ref when an expired ref is used", async (ctx) => {
const client = createTestClient({ ctx })
const badRef = ctx.mock.api.ref().ref
const masterRef = ctx.mock.api.ref().ref
const queryResponse = ctx.mock.api.query({
documents: [ctx.mock.value.document()],
})

const triedRefs = new Set<string | null>()

mockPrismicRestAPIV2({ ctx, queryResponse })
const endpoint = new URL(
"documents/search",
`${client.documentAPIEndpoint}/`,
).toString()
ctx.server.use(
rest.get(endpoint, (req) => {
triedRefs.add(req.url.searchParams.get("ref"))
}),
rest.get(endpoint, (_req, res, ctx) =>
res.once(
ctx.json({ message: `Master ref is: ${masterRef}` }),
ctx.status(410),
),
),
)

const consoleWarnSpy = vi
.spyOn(console, "warn")
.mockImplementation(() => void 0)
await args.run(client, { ref: badRef })
consoleWarnSpy.mockRestore()

expect([...triedRefs]).toStrictEqual([badRef, masterRef])
})

it("throws if the maximum number of retries with invalid refs is reached", async (ctx) => {
const client = createTestClient({ ctx })
const queryResponse = ctx.mock.api.query({
documents: [ctx.mock.value.document()],
})

const triedRefs = new Set<string | null>()

mockPrismicRestAPIV2({ ctx, queryResponse })
const endpoint = new URL(
"documents/search",
`${client.documentAPIEndpoint}/`,
).toString()
ctx.server.use(
rest.get(endpoint, (req) => {
triedRefs.add(req.url.searchParams.get("ref"))
}),
rest.get(endpoint, (_req, res, requestCtx) =>
res(
requestCtx.json({
type: "api_notfound_error",
message: `Master ref is: ${ctx.mock.api.ref().ref}`,
}),
requestCtx.status(404),
),
),
)

const consoleWarnSpy = vi
.spyOn(console, "warn")
.mockImplementation(() => void 0)
await expect(async () => {
await args.run(client)
}).rejects.toThrow(prismic.RefNotFoundError)
consoleWarnSpy.mockRestore()

expect(triedRefs.size).toBe(3)
})

it("fetches a new master ref on subsequent queries if an invalid ref is used", async (ctx) => {
const client = createTestClient({ ctx })
const queryResponse = ctx.mock.api.query({
documents: [ctx.mock.value.document()],
})

const triedRefs = new Set<string | null>()

mockPrismicRestAPIV2({ ctx, queryResponse })
const endpoint = new URL(
"documents/search",
`${client.documentAPIEndpoint}/`,
).toString()
ctx.server.use(
rest.get(endpoint, (req) => {
triedRefs.add(req.url.searchParams.get("ref"))
}),
rest.get(endpoint, (_req, res, requestCtx) =>
res.once(
requestCtx.json({
type: "api_notfound_error",
message: `Master ref is: ${ctx.mock.api.ref().ref}`,
}),
requestCtx.status(404),
),
),
)

const consoleWarnSpy = vi
.spyOn(console, "warn")
.mockImplementation(() => void 0)
await args.run(client)
consoleWarnSpy.mockRestore()

await args.run(client)

expect(triedRefs.size).toBe(3)
})
}
5 changes: 5 additions & 0 deletions test/client-get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { mockPrismicRestAPIV2 } from "./__testutils__/mockPrismicRestAPIV2"
import { testAbortableMethod } from "./__testutils__/testAbortableMethod"
import { testGetMethod } from "./__testutils__/testAnyGetMethod"
import { testFetchOptions } from "./__testutils__/testFetchOptions"
import { testInvalidRefRetry } from "./__testutils__/testInvalidRefRetry"

testGetMethod("resolves a query", {
run: (client) => client.get(),
Expand Down Expand Up @@ -92,6 +93,10 @@ it("uses cached repository metadata within the client's repository cache TTL", a
)
})

testInvalidRefRetry({
run: (client, params) => client.get(params),
})

testFetchOptions("supports fetch options", {
run: (client, params) => client.get(params),
})
Expand Down
5 changes: 5 additions & 0 deletions test/client-getAllByEveryTag.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { testAbortableMethod } from "./__testutils__/testAbortableMethod"
import { testGetAllMethod } from "./__testutils__/testAnyGetMethod"
import { testConcurrentMethod } from "./__testutils__/testConcurrentMethod"
import { testFetchOptions } from "./__testutils__/testFetchOptions"
import { testInvalidRefRetry } from "./__testutils__/testInvalidRefRetry"

testGetAllMethod("returns all documents by every tag from paginated response", {
run: (client) => client.getAllByEveryTag(["foo", "bar"]),
Expand Down Expand Up @@ -29,6 +30,10 @@ testFetchOptions("supports fetch options", {
run: (client, params) => client.getAllByEveryTag(["foo", "bar"], params),
})

testInvalidRefRetry({
run: (client, params) => client.getAllByEveryTag(["foo", "bar"], params),
})

testAbortableMethod("is abortable with an AbortController", {
run: (client, params) => client.getAllByEveryTag(["foo", "bar"], params),
})
Expand Down
5 changes: 5 additions & 0 deletions test/client-getAllByIDs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { testAbortableMethod } from "./__testutils__/testAbortableMethod"
import { testGetAllMethod } from "./__testutils__/testAnyGetMethod"
import { testConcurrentMethod } from "./__testutils__/testConcurrentMethod"
import { testFetchOptions } from "./__testutils__/testFetchOptions"
import { testInvalidRefRetry } from "./__testutils__/testInvalidRefRetry"

testGetAllMethod("returns all documents by IDs from paginated response", {
run: (client) => client.getAllByIDs(["id1", "id2"]),
Expand Down Expand Up @@ -29,6 +30,10 @@ testFetchOptions("supports fetch options", {
run: (client, params) => client.getAllByIDs(["id1", "id2"], params),
})

testInvalidRefRetry({
run: (client, params) => client.getAllByIDs(["id1", "id2"], params),
})

testAbortableMethod("is abortable with an AbortController", {
run: (client, params) => client.getAllByIDs(["id1", "id2"], params),
})
Expand Down
5 changes: 5 additions & 0 deletions test/client-getAllBySomeTags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { testAbortableMethod } from "./__testutils__/testAbortableMethod"
import { testGetAllMethod } from "./__testutils__/testAnyGetMethod"
import { testConcurrentMethod } from "./__testutils__/testConcurrentMethod"
import { testFetchOptions } from "./__testutils__/testFetchOptions"
import { testInvalidRefRetry } from "./__testutils__/testInvalidRefRetry"

testGetAllMethod("returns all documents by some tags from paginated response", {
run: (client) => client.getAllBySomeTags(["foo", "bar"]),
Expand Down Expand Up @@ -29,6 +30,10 @@ testFetchOptions("supports fetch options", {
run: (client, params) => client.getAllBySomeTags(["foo", "bar"], params),
})

testInvalidRefRetry({
run: (client, params) => client.getAllBySomeTags(["foo", "bar"], params),
})

testAbortableMethod("is abortable with an AbortController", {
run: (client, params) => client.getAllBySomeTags(["foo", "bar"], params),
})
Expand Down
5 changes: 5 additions & 0 deletions test/client-getAllByTag.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { testAbortableMethod } from "./__testutils__/testAbortableMethod"
import { testGetAllMethod } from "./__testutils__/testAnyGetMethod"
import { testConcurrentMethod } from "./__testutils__/testConcurrentMethod"
import { testFetchOptions } from "./__testutils__/testFetchOptions"
import { testInvalidRefRetry } from "./__testutils__/testInvalidRefRetry"

testGetAllMethod("returns all documents by tag from paginated response", {
run: (client) => client.getAllByTag("tag"),
Expand Down Expand Up @@ -29,6 +30,10 @@ testFetchOptions("supports fetch options", {
run: (client, params) => client.getAllByTag("tag", params),
})

testInvalidRefRetry({
run: (client, params) => client.getAllByTag("tag", params),
})

testAbortableMethod("is abortable with an AbortController", {
run: (client, params) => client.getAllByTag("tag", params),
})
Expand Down
Loading
Loading