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

[useResponseCache] Ensure calls to cache are awaited #2241

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
6 changes: 6 additions & 0 deletions .changeset/old-moles-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@envelop/response-cache': patch
---

Fixes an issue where rejected promises returned from the cache were not being handled correctly,
causing an unhandled promise rejection which would terminate the process.
17 changes: 10 additions & 7 deletions packages/plugins/response-cache/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,13 +472,13 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
}
}

function invalidateCache(
async function invalidateCache(
result: ExecutionResult,
setResult: (newResult: ExecutionResult) => void,
): void {
): Promise<void> {
processResult(result.data);

cache.invalidate(identifier.values());
await cache.invalidate(identifier.values());
if (includeExtensionMetadata) {
setResult(
resultWithMetadata(result, {
Expand Down Expand Up @@ -535,7 +535,7 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
});
}

function maybeCacheResult(
async function maybeCacheResult(
result: ExecutionResult,
setResult: (newResult: ExecutionResult) => void,
) {
Expand All @@ -550,7 +550,7 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
return;
}

cache.set(cacheKey, result, identifier.values(), finalTtl);
await cache.set(cacheKey, result, identifier.values(), finalTtl);
if (includeExtensionMetadata) {
setResult(resultWithMetadata(result, { hit: false, didCache: true, ttl: finalTtl }));
}
Expand Down Expand Up @@ -580,7 +580,10 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
}

function handleAsyncIterableResult<PluginContext extends Record<string, any> = {}>(
handler: (result: ExecutionResult, setResult: (newResult: ExecutionResult) => void) => void,
handler: (
result: ExecutionResult,
setResult: (newResult: ExecutionResult) => void,
) => void | Promise<void>,
): OnExecuteDoneHookResult<PluginContext> {
// When the result is an AsyncIterable, it means the query is using @defer or @stream.
// This means we have to build the final result by merging the incremental results.
Expand Down Expand Up @@ -612,7 +615,7 @@ function handleAsyncIterableResult<PluginContext extends Record<string, any> = {

if (!hasNext) {
// The query is complete, we can process the final result
handler(result, payload.setResult);
return handler(result, payload.setResult);
}
}
},
Expand Down
230 changes: 230 additions & 0 deletions packages/plugins/response-cache/test/response-cache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3977,6 +3977,236 @@ describe('useResponseCache', () => {
expect(result).toEqual({ data: { users: [{ id: '1' }] } });
});

it('handles errors thrown from an asynchronous cache set', async () => {
const schema = makeExecutableSchema({
typeDefs: /* GraphQL */ `
type Query {
users: [User!]!
}

type User {
id: ID!
}
`,
resolvers: {
Query: {
users: () => [{ id: 1 }],
},
},
});

const cache: Cache = {
get: jest.fn(),
invalidate: jest.fn(),
set: jest.fn(() => Promise.reject(new Error('Failed to set cache'))),
};

const testInstance = createTestkit(
[
useResponseCache({
session: () => null,
cache,
}),
],
schema,
);

const query = /* GraphQL */ `
query test {
users {
id
}
}
`;

await expect(testInstance.execute(query)).rejects.toThrow('Failed to set cache');
expect(cache.set).toHaveBeenCalledTimes(1);
});

it('handles errors thrown from an asynchronous cache invalidate', async () => {
const schema = makeExecutableSchema({
typeDefs: /* GraphQL */ `
type Query {
users: [User!]!
}

type Mutation {
updateUser(id: ID!): User!
}

type User {
id: ID!
}
`,
resolvers: {
Query: {
users: () => [{ id: 1 }],
},
Mutation: {
updateUser(_, { id }) {
return {
id,
};
},
},
},
});

const cache: Cache = {
get: jest.fn(),
set: jest.fn(),
invalidate: jest.fn(() => Promise.reject(new Error('Failed to invalidate cache'))),
};

const testInstance = createTestkit(
[
useResponseCache({
session: () => null,
cache,
}),
],
schema,
);

const query = /* GraphQL */ `
query test {
users {
id
}
}
`;

await waitForResult(testInstance.execute(query));

const mutation = /* GraphQL */ `
mutation {
updateUser(id: "1") {
id
}
}
`;

await expect(testInstance.execute(mutation)).rejects.toThrow('Failed to invalidate cache');
expect(cache.invalidate).toHaveBeenCalledTimes(1);
});

it('handles errors thrown from a synchronous cache set', async () => {
const schema = makeExecutableSchema({
typeDefs: /* GraphQL */ `
type Query {
users: [User!]!
}

type User {
id: ID!
}
`,
resolvers: {
Query: {
users: () => [{ id: 1 }],
},
},
});

const cache: Cache = {
get: jest.fn(),
invalidate: jest.fn(),
set: jest.fn(() => {
throw new Error('Failed to set cache');
}),
};

const testInstance = createTestkit(
[
useResponseCache({
session: () => null,
cache,
}),
],
schema,
);

const query = /* GraphQL */ `
query test {
users {
id
}
}
`;

await expect(testInstance.execute(query)).rejects.toThrow('Failed to set cache');
expect(cache.set).toHaveBeenCalledTimes(1);
});

it('handles errors thrown from a synchronous cache invalidate', async () => {
const schema = makeExecutableSchema({
typeDefs: /* GraphQL */ `
type Query {
users: [User!]!
}

type Mutation {
updateUser(id: ID!): User!
}

type User {
id: ID!
}
`,
resolvers: {
Query: {
users: () => [{ id: 1 }],
},
Mutation: {
updateUser(_, { id }) {
return {
id,
};
},
},
},
});

const cache: Cache = {
get: jest.fn(),
set: jest.fn(),
invalidate: jest.fn(() => {
throw new Error('Failed to invalidate cache');
}),
};

const testInstance = createTestkit(
[
useResponseCache({
session: () => null,
cache,
}),
],
schema,
);

const query = /* GraphQL */ `
query test {
users {
id
}
}
`;

await waitForResult(testInstance.execute(query));

const mutation = /* GraphQL */ `
mutation {
updateUser(id: "1") {
id
}
}
`;

await expect(testInstance.execute(mutation)).rejects.toThrow('Failed to invalidate cache');
expect(cache.invalidate).toHaveBeenCalledTimes(1);
});

async function waitForResult(result: any) {
result = await result;
if (result.next) {
Expand Down