Skip sending initial request with state if we know the request body is over the limit of 200KB

Author: ndbroadbent

.eslintignore

@@ -1,2 +1,3 @@
 built/
 coverage/
+vendor/

index.js

@@ -1,3 +1,5 @@
+const stringify = require("./vendor/json-stringify-safe/stringify");
+
 const identity = x => x;
 const getUndefined = () => {};
 const filter = () => true;
@@ -32,12 +34,12 @@ function createRavenMiddleware(Raven, options = {}) {
       return original ? original(data) : data;
     });
 
-    const retryCaptureWithoutReduxState = captureFn => {
+    const retryCaptureWithoutReduxState = (errorMessage, captureFn) => {
       Raven.setDataCallback((data, originalCallback) => {
         Raven.setDataCallback(originalCallback);
         const reduxExtra = {
           lastAction: actionTransformer(lastAction),
-          state: "Failed to submit state to Sentry: 413 request too large."
+          state: errorMessage
         };
         data.extra = Object.assign(reduxExtra, data.extra);
         data.breadcrumbs.values = [];
@@ -50,15 +52,28 @@ function createRavenMiddleware(Raven, options = {}) {
       Raven._globalOptions.allowDuplicates = originalAllowDuplicates;
     };
 
-    const retryWithoutStateOnRequestTooLarge = originalFn => {
+    const retryWithoutStateIfRequestTooLarge = originalFn => {
       return (...captureArguments) => {
         const originalTransport = Raven._globalOptions.transport;
         Raven.setTransport(opts => {
           Raven.setTransport(originalTransport);
+          const requestBody = stringify(opts.data);
+          if (requestBody.length > 200000) {
+            // We know the request is too large, so don't try sending it to Sentry.
+            // Retry the capture function, and don't include the state this time.
+            const errorMessage =
+              "Could not send state because request would be larger than 200KB. " +
+              `(Was: ${requestBody.length}B)`;
+            retryCaptureWithoutReduxState(errorMessage, () => {
+              originalFn.apply(Raven, captureArguments);
+            });
+            return;
+          }
           opts.onError = error => {
             if (error.request && error.request.status === 413) {
-              // Retry request without state after "413 request too large" error
-              retryCaptureWithoutReduxState(() => {
+              const errorMessage =
+                "Failed to submit state to Sentry: 413 request too large.";
+              retryCaptureWithoutReduxState(errorMessage, () => {
                 originalFn.apply(Raven, captureArguments);
               });
             }
@@ -69,10 +84,10 @@ function createRavenMiddleware(Raven, options = {}) {
       };
     };
 
-    Raven.captureException = retryWithoutStateOnRequestTooLarge(
+    Raven.captureException = retryWithoutStateIfRequestTooLarge(
       Raven.captureException
     );
-    Raven.captureMessage = retryWithoutStateOnRequestTooLarge(
+    Raven.captureMessage = retryWithoutStateIfRequestTooLarge(
       Raven.captureMessage
     );
 

index.test.js

@@ -2,6 +2,12 @@ const Raven = require("raven-js");
 const createRavenMiddleware = require("./index");
 const { createStore, applyMiddleware } = require("redux");
 
+const stringify = require.requireActual(
+  "./vendor/json-stringify-safe/stringify"
+);
+const stringifyMocked = require("./vendor/json-stringify-safe/stringify");
+jest.mock("./vendor/json-stringify-safe/stringify");
+
 Raven.config(
   "https://5d5bf17b1bed4afc9103b5a09634775e@sentry.io/146969"
 ).install();
@@ -27,6 +33,7 @@ const context = {};
 
 describe("raven-for-redux", () => {
   beforeEach(() => {
+    stringifyMocked.mockImplementation(obj => stringify(obj));
     context.mockTransport = jest.fn();
     Raven.setTransport(context.mockTransport);
     Raven.setDataCallback(undefined);
@@ -188,13 +195,48 @@ describe("raven-for-redux", () => {
     });
 
     ["captureException", "captureMessage"].forEach(fnName => {
+      it(`skips state for ${fnName} if state is larger than 200000B`, () => {
+        expect(Raven._globalOptions.transport).toEqual(context.mockTransport);
+        stringifyMocked
+          .mockClear()
+          .mockImplementationOnce(() => ({ length: 200001 }))
+          .mockImplementationOnce(() => ({ length: 500 }));
+        // Test that allowDuplicates is set to true inside our handler and reset afterwards
+        // (Error message needs to be unique for each test, because we set allowDuplicates to null)
+        Raven._globalOptions.allowDuplicates = null;
+        Raven[fnName].call(
+          Raven,
+          fnName === "captureException"
+            ? new Error("Test skip state")
+            : "Test skip state"
+        );
+
+        // Ensure transport and allowDuplicates have been reset
+        expect(Raven._globalOptions.transport).toEqual(context.mockTransport);
+        expect(Raven._globalOptions.allowDuplicates).toEqual(null);
+        expect(context.mockTransport).toHaveBeenCalledTimes(1);
+        const { extra } = context.mockTransport.mock.calls[0][0].data;
+        expect(extra).toMatchObject({
+          state:
+            "Could not send state because request would be larger than 200KB. (Was: 200001B)",
+          lastAction: undefined
+        });
+      });
+
       it(`retries ${fnName} without any state if Sentry returns 413 request too large`, () => {
+        expect(Raven._globalOptions.transport).toEqual(context.mockTransport);
         context.mockTransport.mockImplementationOnce(options => {
           options.onError({ request: { status: 413 } });
         });
-        // allowDuplicates is set to true inside our handler and reset afterwards
+        // Test that allowDuplicates is set to true inside our handler and reset afterwards
+        // (Error message needs to be unique for each test, because we set allowDuplicates to null)
         Raven._globalOptions.allowDuplicates = null;
-        Raven[fnName].call(Raven, new Error("Crash!"));
+        Raven[fnName].call(
+          Raven,
+          fnName === "captureException"
+            ? new Error("Test retry on 413 error")
+            : "Test retry on 413 error"
+        );
 
         // Ensure transport and allowDuplicates have been reset
         expect(Raven._globalOptions.transport).toEqual(context.mockTransport);

package.json

@@ -6,7 +6,7 @@
   "scripts": {
     "fix": "eslint --fix .",
     "tdd": "jest --watch",
-    "test": "jest --coverage && npm run lint",
+    "test": "jest --coverage --collectCoverageFrom=index.js && npm run lint",
     "lint": "eslint .",
     "build": "babel index.js --out-dir built/",
     "prepublishOnly": "npm run test && npm run build",

vendor/json-stringify-safe/stringify.js

@@ -0,0 +1,76 @@
+/*
+ json-stringify-safe
+ Like JSON.stringify, but doesn't throw on circular references.
+
+ Copied from sentry-javascript/packages/raven-js/vendor/json-stringify-safe/stringify.js
+
+ Originally forked from https://github.com/isaacs/json-stringify-safe
+ version 5.0.1 on 3/8/2017 and modified to handle Errors serialization
+ and IE8 compatibility. Tests for this are in test/vendor.
+
+ ISC license: https://github.com/isaacs/json-stringify-safe/blob/master/LICENSE
+*/
+
+exports = module.exports = stringify;
+exports.getSerialize = serializer;
+
+function indexOf(haystack, needle) {
+  for (var i = 0; i < haystack.length; ++i) {
+    if (haystack[i] === needle) return i;
+  }
+  return -1;
+}
+
+function stringify(obj, replacer, spaces, cycleReplacer) {
+  return JSON.stringify(obj, serializer(replacer, cycleReplacer), spaces);
+}
+
+// https://github.com/ftlabs/js-abbreviate/blob/fa709e5f139e7770a71827b1893f22418097fbda/index.js#L95-L106
+function stringifyError(value) {
+  var err = {
+    // These properties are implemented as magical getters and don't show up in for in
+    stack: value.stack,
+    message: value.message,
+    name: value.name
+  };
+
+  for (var i in value) {
+    if (Object.prototype.hasOwnProperty.call(value, i)) {
+      err[i] = value[i];
+    }
+  }
+
+  return err;
+}
+
+function serializer(replacer, cycleReplacer) {
+  var stack = [];
+  var keys = [];
+
+  if (cycleReplacer == null) {
+    cycleReplacer = function(key, value) {
+      if (stack[0] === value) {
+        return '[Circular ~]';
+      }
+      return '[Circular ~.' + keys.slice(0, indexOf(stack, value)).join('.') + ']';
+    };
+  }
+
+  return function(key, value) {
+    if (stack.length > 0) {
+      var thisPos = indexOf(stack, this);
+      ~thisPos ? stack.splice(thisPos + 1) : stack.push(this);
+      ~thisPos ? keys.splice(thisPos, Infinity, key) : keys.push(key);
+
+      if (~indexOf(stack, value)) {
+        value = cycleReplacer.call(this, key, value);
+      }
+    } else {
+      stack.push(value);
+    }
+
+    return replacer == null
+      ? value instanceof Error ? stringifyError(value) : value
+      : replacer.call(this, key, value);
+  };
+}