Skip to content

Commit

Permalink
feat: retry invalid/expired refs (#356)
Browse files Browse the repository at this point in the history
* feat: retry invalid/expired refs

* fix: remove duplicate ref retry logic and support `getFirst`

* test: invalid ref retry

* feat: allow up to 3 retries before throwing

* feat: use a new master ref once a known-stale ref is used

* test: simplify test title

* docs: update const description
  • Loading branch information
angeloashmore authored Oct 30, 2024
1 parent 9da8fdf commit ee06efa
Show file tree
Hide file tree
Showing 19 changed files with 330 additions and 7 deletions.
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

0 comments on commit ee06efa

Please sign in to comment.