fix(middleware): Correlate responses by index when server does not re…

…turn ID (#56) closes #55
relay-tools · Nov 24, 2017 · 3af7e69 · 3af7e69
1 parent 5a1970f
commit 3af7e69
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 5 deletions.
diff --git a/src/middleware/__tests__/batch.test.js b/src/middleware/__tests__/batch.test.js
@@ -38,6 +38,44 @@ describe('batchMiddleware', () => {
     expect(req2.payload).toEqual({ response: { ok: 2 } });
   });
 
+  it('should make a successfully batch request without server IDs', async () => {
+    fetchMock.mock({
+      matcher: '/graphql/batch',
+      response: {
+        status: 200,
+        body: [{ data: { ok: 1 } }, { data: { ok: 2 } }],
+      },
+      method: 'POST',
+    });
+
+    const req1 = mockReq(1);
+    const req2 = mockReq(2);
+    await rnl.sendQueries([req1, req2]);
+    expect(req1.payload).toEqual({ response: { ok: 1 } });
+    expect(req2.payload).toEqual({ response: { ok: 2 } });
+  });
+
+  it('should reject if server returns a different number of responses than requests', async () => {
+    fetchMock.mock({
+      matcher: '/graphql/batch',
+      response: {
+        status: 200,
+        body: [{ data: { ok: 2 } }],
+      },
+      method: 'POST',
+    });
+
+    const req1 = mockReq(1);
+    const req2 = mockReq(2);
+    await rnl.sendQueries([req1, req2]).catch(() => {});
+    expect(req1.error.toString()).toMatch(
+      'Server returned a different number of responses than requested.'
+    );
+    expect(req2.error.toString()).toMatch(
+      'Server returned a different number of responses than requested.'
+    );
+  });
+
   it('should make a successfully batch request with duplicate request ids', async () => {
     fetchMock.mock({
       matcher: '/graphql/batch',
@@ -63,7 +101,7 @@ describe('batchMiddleware', () => {
       matcher: '/graphql/batch',
       response: {
         status: 200,
-        body: [{ data: {} }, { id: 2, data: { ok: 2 } }],
+        body: [{ id: 2, data: { ok: 2 } }],
       },
       method: 'POST',
     });
@@ -82,14 +120,14 @@ describe('batchMiddleware', () => {
       matcher: '/graphql/batch',
       response: {
         status: 200,
-        body: [{ data: {} }, { id: 2, data: { ok: 2 } }],
+        body: [{ id: 2, data: { ok: 2 } }],
       },
       method: 'POST',
     });
 
     const req1 = mockReq(1);
     const req2 = mockReq(2);
-    const req3 = mockReq(3);
+    const req3 = mockReq(1);
     await rnl.sendQueries([req1, req2, req3]).catch(() => {});
 
     expect(req1.error).toBeInstanceOf(Error);

diff --git a/src/middleware/batch.js b/src/middleware/batch.js
@@ -174,9 +174,16 @@ function sendRequests(requestMap: BatchRequestMap, next, opts) {
           throw new Error('Wrong response from server');
         }
 
-        batchResponse.payload.forEach(res => {
+        const responseHasIds: boolean = batchResponse.payload.every(response => response.id);
+        if (!responseHasIds && ids.length !== batchResponse.payload.length) {
+          throw new Error(`Server returned a different number of responses than requested.
+            It's not possible to correlate requests and responses`);
+        }
+
+        batchResponse.payload.forEach((res, i) => {
           if (!res) return;
-          const request = requestMap[res.id];
+          const request = responseHasIds ? requestMap[res.id] : requestMap[ids[i]];
+
           if (request) {
             const responsePayload = copyBatchResponse(batchResponse, res);
             request.completeOk(responsePayload);