Skip to content

Commit

Permalink
fix: issue where query is stuck on loading if final chunk contains on…
Browse files Browse the repository at this point in the history
…ly hasNext: false
  • Loading branch information
alessbell committed Apr 6, 2023
1 parent 4bd3dac commit a58b528
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/serious-feet-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

If a multipart chunk contains only `hasNext: false`, immediately complete the observable.
77 changes: 77 additions & 0 deletions src/link/http/__tests__/HttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,18 @@ describe('HttpLink', () => {
'-----',
].join("\r\n");

const nonNullErrorBody = [
"--graphql",
"content-type: application/json",
"",
'{"data":{"allProducts":[null,null,null]},"errors":[{"message":"Cannot return null for non-nullable field Product.nonNullErrorField.","locations":[{"line":1,"column":53}],"path":["allProducts",0,"nonNullErrorField"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: Cannot return null for non-nullable field Product.nonNullErrorField."," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:594:13)"," at executeField (/usr/src/app/node_modules/graphql/execution/execute.js:489:19)"," at executeFields (/usr/src/app/node_modules/graphql/execution/execute.js:413:20)"," at completeObjectValue (/usr/src/app/node_modules/graphql/execution/execute.js:914:10)"," at completeAbstractValue (/usr/src/app/node_modules/graphql/execution/execute.js:795:10)"," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:624:12)"," at /usr/src/app/node_modules/graphql/execution/execute.js:696:25"," at Function.from (<anonymous>)"," at completeListValue (/usr/src/app/node_modules/graphql/execution/execute.js:676:34)"," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:607:12)"]}}},{"message":"Cannot return null for non-nullable field Product.nonNullErrorField.","locations":[{"line":1,"column":53}],"path":["allProducts",1,"nonNullErrorField"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: Cannot return null for non-nullable field Product.nonNullErrorField."," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:594:13)"," at executeField (/usr/src/app/node_modules/graphql/execution/execute.js:489:19)"," at executeFields (/usr/src/app/node_modules/graphql/execution/execute.js:413:20)"," at completeObjectValue (/usr/src/app/node_modules/graphql/execution/execute.js:914:10)"," at completeAbstractValue (/usr/src/app/node_modules/graphql/execution/execute.js:795:10)"," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:624:12)"," at /usr/src/app/node_modules/graphql/execution/execute.js:696:25"," at Function.from (<anonymous>)"," at completeListValue (/usr/src/app/node_modules/graphql/execution/execute.js:676:34)"," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:607:12)"]}}},{"message":"Cannot return null for non-nullable field Product.nonNullErrorField.","locations":[{"line":1,"column":53}],"path":["allProducts",2,"nonNullErrorField"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: Cannot return null for non-nullable field Product.nonNullErrorField."," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:594:13)"," at executeField (/usr/src/app/node_modules/graphql/execution/execute.js:489:19)"," at executeFields (/usr/src/app/node_modules/graphql/execution/execute.js:413:20)"," at completeObjectValue (/usr/src/app/node_modules/graphql/execution/execute.js:914:10)"," at completeAbstractValue (/usr/src/app/node_modules/graphql/execution/execute.js:795:10)"," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:624:12)"," at /usr/src/app/node_modules/graphql/execution/execute.js:696:25"," at Function.from (<anonymous>)"," at completeListValue (/usr/src/app/node_modules/graphql/execution/execute.js:676:34)"," at completeValue (/usr/src/app/node_modules/graphql/execution/execute.js:607:12)"]}}}],"hasNext":true}',
"--graphql",
"content-type: application/json",
"",
'{"hasNext":false}',
"--graphql--",
].join("\r\n");

it('whatwg stream bodies', (done) => {
const stream = new ReadableStream({
async start(controller) {
Expand Down Expand Up @@ -1418,6 +1430,71 @@ describe('HttpLink', () => {
);
});

it('whatwg stream bodies, non-null errors', (done) => {
const stream = new ReadableStream({
async start(controller) {
const lines = nonNullErrorBody.split("\r\n");
try {
for (const line of lines) {
await new Promise((resolve) => setTimeout(resolve, 10));
controller.enqueue(line + "\r\n");
}
} finally {
controller.close();
}
},
});

const fetch = jest.fn(async () => ({
status: 200,
body: stream,
headers: new Headers({ 'Content-Type': 'multipart/mixed;boundary="graphql";deferSpec=20220824' }),
}));

const link = new HttpLink({
fetch: fetch as any,
});

let i = 0;
execute(link, { query: sampleDeferredQuery }).subscribe(
result => {
try {
if (i === 0) {
expect(result).toMatchObject({
data: {
allProducts: [
null,
null,
null
]
},
// errors is also present, but for the purpose of this test
// we're not interested in its (lengthy) content.
// errors: [{...}],
hasNext: true,
});
}
// Since the second chunk contains only hasNext: false,
// there is no next result to receive.
} catch (err) {
done(err);
} finally {
i++;
}
},
err => {
done(err);
},
() => {
if (i !== 1) {
done(new Error("Unexpected end to observable"));
}

done();
},
);
});

it('node stream bodies', (done) => {
const stream = Readable.from(body.split("\r\n").map((line) => line + "\r\n"));

Expand Down
8 changes: 8 additions & 0 deletions src/link/http/parseAndCheckHttpResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ export async function readMultipartBody<
// we don't need to call observer.next as there is no data/errors
observer.next?.(result);
}
} else if (
// If the chunk contains only a "hasNext: false", we can call
// observer.complete() immediately.
Object.keys(result).length === 1 &&
"hasNext" in result &&
!result.hasNext
) {
observer.complete?.();
}
} catch (err) {
handleError(err, observer);
Expand Down

0 comments on commit a58b528

Please sign in to comment.