fix: Bunch of small fixes, refactoring, cleanups

Author: nodkz

README.md

@@ -1,7 +1,7 @@
 ReactRelayNetworkLayer
 ======================
 [![](https://img.shields.io/npm/v/react-relay-network-layer.svg)](https://www.npmjs.com/package/react-relay-network-layer)
-[![npm](https://img.shields.io/npm/dt/react-relay-network-layer.svg)](https://www.npmjs.com/package/react-relay-network-layer)
+[![npm](https://img.shields.io/npm/dt/react-relay-network-layer.svg)](http://www.npmtrends.com/react-relay-network-layer)
 [![Travis](https://img.shields.io/travis/nodkz/react-relay-network-layer.svg?maxAge=2592000)](https://travis-ci.org/nodkz/react-relay-network-layer)
 [![Commitizen friendly](https://img.shields.io/badge/commitizen-friendly-brightgreen.svg)](http://commitizen.github.io/cz-cli/)
 [![semantic-release](https://img.shields.io/badge/%20%20%F0%9F%93%A6%F0%9F%9A%80-semantic--release-e10079.svg)](https://github.com/semantic-release/semantic-release)
@@ -43,7 +43,7 @@ Previous documentation for version 1.x.x can be found [here](https://github.com/
   - `batchUrl` - string or function(requestMap). Url of the server endpoint for batch request execution (default: `/graphql/batch`)
   - `batchTimeout` - integer in milliseconds, period of time for gathering multiple requests before sending them to the server (default: `0`)
   - `allowMutations` - by default batching disabled for mutations, you may enable it passing `true` (default: `false`)
-  - `maxBatchSize` - integer representing maximum size of request to be sent in a single batch. Once a request hits the provided size in length a new batch request is ran. (default: 102400 (roughly 100kb))
+  - `maxBatchSize` - integer representing maximum size of request to be sent in a single batch. Once a request hits the provided size in length a new batch request is ran. (default: `102400` characters, roughly 100kb for 1-byte characters or 200kb for 2-byte characters)
 - **retryMiddleware** - for request retry if the initial request fails.
   - `fetchTimeout` - number in milliseconds that defines in how much time will request timeout after it has been sent to the server (default: `15000`).
   - `retryDelays` - array of millisecond that defines the values on which retries are based on (default: `[1000, 3000]`).

src/createRequestError.js

@@ -0,0 +1,77 @@
+/**
+ * Formats an error response from GraphQL server request.
+ */
+function formatRequestErrors(request, errors) {
+  const CONTEXT_BEFORE = 20;
+  const CONTEXT_LENGTH = 60;
+
+  if (!request.getQueryString) {
+    return errors.join('\n');
+  }
+
+  const queryLines = request.getQueryString().split('\n');
+  return errors
+    .map(({ locations, message }, ii) => {
+      const prefix = `${ii + 1}. `;
+      const indent = ' '.repeat(prefix.length);
+
+      // custom errors thrown in graphql-server may not have locations
+      const locationMessage = locations
+        ? '\n' +
+            locations
+              .map(({ column, line }) => {
+                const queryLine = queryLines[line - 1];
+                const offset = Math.min(column - 1, CONTEXT_BEFORE);
+                return [
+                  queryLine.substr(column - 1 - offset, CONTEXT_LENGTH),
+                  `${' '.repeat(Math.max(offset, 0))}^^^`,
+                ]
+                  .map(messageLine => indent + messageLine)
+                  .join('\n');
+              })
+              .join('\n')
+        : '';
+      return prefix + message + locationMessage;
+    })
+    .join('\n');
+}
+
+export default function createRequestError(
+  request,
+  requestType,
+  responseStatus,
+  payload
+) {
+  let errorReason;
+  if (typeof payload === 'object' && payload.errors) {
+    errorReason = formatRequestErrors(request, payload.errors);
+  } else if (payload) {
+    errorReason = payload;
+  } else {
+    errorReason = `Server response had an error status: ${responseStatus}`;
+  }
+
+  let error;
+  if (request.getDebugName) {
+    // for native Relay request
+    error = new Error(
+      `Server request for ${requestType} \`${request.getDebugName()}\` ` +
+        `failed for the following reasons:\n\n${errorReason}`
+    );
+  } else if (request && typeof request === 'object') {
+    // for batch request
+    const ids = Object.keys(request);
+    error = new Error(
+      `Server request for \`BATCH_QUERY:${ids.join(':')}\` ` +
+        `failed for the following reasons:\n\n${errorReason}`
+    );
+  } else {
+    error = new Error(
+      `Server request failed for the following reasons:\n\n${errorReason}`
+    );
+  }
+
+  error.source = payload;
+  error.status = responseStatus;
+  return error;
+}

src/fetchWrapper.js renamed to src/fetchWithMiddleware.js

@@ -1,6 +1,8 @@
 /* eslint-disable no-param-reassign */
 
-export default function fetchWrapper(reqFromRelay, middlewares) {
+import createRequestError from './createRequestError';
+
+export default function fetchWithMiddleware(reqFromRelay, middlewares) {
   const fetchAfterAllWrappers = req => {
     let { url, ...opts } = req;
 
@@ -12,27 +14,46 @@ export default function fetchWrapper(reqFromRelay, middlewares) {
       }
     }
 
-    return fetch(url, opts).then(res => {
-      // sub-promise for combining `res` with parsed json
-      return res
-        .json()
-        .then(json => {
-          res.json = json;
+    return fetch(url, opts)
+      .then(res => {
+        if (res.status >= 200 && res.status < 300) {
           return res;
-        })
-        .catch(e => {
-          console.warn('error parsing response json', e); // eslint-disable-line no-console
-          res.json = {};
+        }
+        return res.text().then(text => {
+          const err = new Error(text);
+          err.fetchResponse = res;
+          throw err;
+        });
+      })
+      .then(res => {
+        // sub-promise for combining `res` with parsed json
+        return res.json().then(json => {
+          res.json = json;
           return res;
         });
-    });
+      });
   };
 
   const wrappedFetch = compose(...middlewares)(fetchAfterAllWrappers);
 
-  return wrappedFetch(reqFromRelay)
-    .then(throwOnServerError)
-    .then(res => res.json);
+  return wrappedFetch(reqFromRelay).then(res => {
+    const payload = res.json;
+    if ({}.hasOwnProperty.call(payload, 'errors')) {
+      throw createRequestError(
+        reqFromRelay.relayReqObj,
+        reqFromRelay.relayReqType,
+        '200',
+        payload
+      );
+    } else if (!{}.hasOwnProperty.call(payload, 'data')) {
+      throw new Error(
+        'Server response.data was missing for query `' +
+          reqFromRelay.relayReqObj.getDebugName() +
+          '`.'
+      );
+    }
+    return payload.data;
+  });
 }
 
 /**
@@ -55,11 +76,3 @@ function compose(...funcs) {
       rest.reduceRight((composed, f) => f(composed), last(...args));
   }
 }
-
-function throwOnServerError(response) {
-  if (response.status >= 200 && response.status < 300) {
-    return response;
-  }
-
-  throw response;
-}

src/formatRequestErrors.js

@@ -5,16 +5,6 @@ import { isFunction } from '../utils';
 // Max out at roughly 100kb (express-graphql imposed max)
 const DEFAULT_BATCH_SIZE = 102400;
 
-function copyBatchResponse(batchResponse, res) {
-  // Supports graphql-js, graphql-graphene and apollo-server responses
-  const json = res.payload ? res.payload : res;
-  return {
-    ok: batchResponse.ok,
-    status: batchResponse.status,
-    json,
-  };
-}
-
 export default function batchMiddleware(opts = {}) {
   const batchTimeout = opts.batchTimeout || 0; // 0 is the same as nextTick in nodeJS
   const allowMutations = opts.allowMutations || false;
@@ -24,7 +14,12 @@ export default function batchMiddleware(opts = {}) {
 
   return next =>
     req => {
-      if (req.relayReqType === 'mutation' && !allowMutations) {
+      // never batch mutations with files
+      // mutation without files can be batched if allowMutations = true
+      if (
+        req.relayReqType === 'mutation' &&
+        (!allowMutations || (global.FormData && req.body instanceof FormData))
+      ) {
         return next(req);
       }
 
@@ -53,7 +48,17 @@ function passThroughBatch(req, next, opts) {
 
   // queue request
   return new Promise((resolve, reject) => {
-    singleton.batcher.requestMap[req.relayReqId] = { req, resolve, reject };
+    singleton.batcher.requestMap[req.relayReqId] = {
+      req,
+      completeOk: res => {
+        req.done = true;
+        resolve(res);
+      },
+      completeErr: err => {
+        req.done = true;
+        reject(err);
+      },
+    };
   });
 }
 
@@ -67,19 +72,9 @@ function prepareNewBatcher(next, opts) {
   setTimeout(
     () => {
       batcher.acceptRequests = false;
-      sendRequests(batcher.requestMap, next, opts).then(() => {
-        // check that server returns responses for all requests
-        Object.keys(batcher.requestMap).forEach(id => {
-          if (!batcher.requestMap[id].done) {
-            batcher.requestMap[id].reject(
-              new Error(
-                `Server does not return response for request with id ${id} \n` +
-                  `eg. { "id": "${id}", "data": {} }`
-              )
-            );
-          }
-        });
-      });
+      sendRequests(batcher.requestMap, next, opts)
+        .then(() => finalizeUncompleted(batcher))
+        .catch(() => finalizeUncompleted(batcher));
     },
     opts.batchTimeout
   );
@@ -95,8 +90,7 @@ function sendRequests(requestMap, next, opts) {
     const request = requestMap[ids[0]];
 
     return next(request.req).then(res => {
-      request.done = true;
-      request.resolve(res);
+      request.completeOk(res);
     });
   } else if (ids.length > 1) {
     // SEND AS BATCHED QUERY
@@ -128,18 +122,43 @@ function sendRequests(requestMap, next, opts) {
           const request = requestMap[res.id];
           if (request) {
             const responsePayload = copyBatchResponse(batchResponse, res);
-            request.done = true;
-            request.resolve(responsePayload);
+            request.completeOk(responsePayload);
           }
         });
       })
       .catch(e => {
         ids.forEach(id => {
-          requestMap[id].done = true;
-          requestMap[id].reject(e);
+          requestMap[id].completeErr(e);
         });
       });
   }
 
   return Promise.resolve();
 }
+
+// check that server returns responses for all requests
+function finalizeUncompleted(batcher) {
+  Object.keys(batcher.requestMap).forEach(id => {
+    if (!batcher.requestMap[id].req.done) {
+      batcher.requestMap[id].completeErr(
+        new Error(
+          `Server does not return response for request with id ${id} \n` +
+            `eg. { "id": "${id}", "data": {} }`
+        )
+      );
+    }
+  });
+}
+
+function copyBatchResponse(batchResponse, res) {
+  // Fallback for graphql-graphene and apollo-server batch responses
+  const json = res.payload || res;
+  return {
+    ok: batchResponse.ok,
+    status: batchResponse.status,
+    statusText: batchResponse.statusText,
+    url: batchResponse.url,
+    headers: batchResponse.headers,
+    json,
+  };
+}

src/middleware/batch.js

@@ -16,7 +16,9 @@ export default function urlMiddleware(opts = {}) {
         }
       }
 
-      req.url = isFunction(urlOrThunk) ? urlOrThunk(req) : urlOrThunk;
+      if (req.relayReqType !== 'batch-query') {
+        req.url = isFunction(urlOrThunk) ? urlOrThunk(req) : urlOrThunk;
+      }
 
       return next(req);
     };