Skip to content

Commit 40e00a4

Browse files
committed
Skip sending initial request with state if we know the request body is over the limit of 200KB
1 parent 734e5c6 commit 40e00a4

File tree

5 files changed

+145
-11
lines changed

5 files changed

+145
-11
lines changed

.eslintignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
built/
22
coverage/
3+
vendor/

index.js

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const stringify = require("./vendor/json-stringify-safe/stringify");
2+
13
const identity = x => x;
24
const getUndefined = () => {};
35
const filter = () => true;
@@ -32,12 +34,12 @@ function createRavenMiddleware(Raven, options = {}) {
3234
return original ? original(data) : data;
3335
});
3436

35-
const retryCaptureWithoutReduxState = captureFn => {
37+
const retryCaptureWithoutReduxState = (errorMessage, captureFn) => {
3638
Raven.setDataCallback((data, originalCallback) => {
3739
Raven.setDataCallback(originalCallback);
3840
const reduxExtra = {
3941
lastAction: actionTransformer(lastAction),
40-
state: "Failed to submit state to Sentry: 413 request too large."
42+
state: errorMessage
4143
};
4244
data.extra = Object.assign(reduxExtra, data.extra);
4345
data.breadcrumbs.values = [];
@@ -50,15 +52,28 @@ function createRavenMiddleware(Raven, options = {}) {
5052
Raven._globalOptions.allowDuplicates = originalAllowDuplicates;
5153
};
5254

53-
const retryWithoutStateOnRequestTooLarge = originalFn => {
55+
const retryWithoutStateIfRequestTooLarge = originalFn => {
5456
return (...captureArguments) => {
5557
const originalTransport = Raven._globalOptions.transport;
5658
Raven.setTransport(opts => {
5759
Raven.setTransport(originalTransport);
60+
const requestBody = stringify(opts.data);
61+
if (requestBody.length > 200000) {
62+
// We know the request is too large, so don't try sending it to Sentry.
63+
// Retry the capture function, and don't include the state this time.
64+
const errorMessage =
65+
"Could not send state because request would be larger than 200KB. " +
66+
`(Was: ${requestBody.length}B)`;
67+
retryCaptureWithoutReduxState(errorMessage, () => {
68+
originalFn.apply(Raven, captureArguments);
69+
});
70+
return;
71+
}
5872
opts.onError = error => {
5973
if (error.request && error.request.status === 413) {
60-
// Retry request without state after "413 request too large" error
61-
retryCaptureWithoutReduxState(() => {
74+
const errorMessage =
75+
"Failed to submit state to Sentry: 413 request too large.";
76+
retryCaptureWithoutReduxState(errorMessage, () => {
6277
originalFn.apply(Raven, captureArguments);
6378
});
6479
}
@@ -69,10 +84,10 @@ function createRavenMiddleware(Raven, options = {}) {
6984
};
7085
};
7186

72-
Raven.captureException = retryWithoutStateOnRequestTooLarge(
87+
Raven.captureException = retryWithoutStateIfRequestTooLarge(
7388
Raven.captureException
7489
);
75-
Raven.captureMessage = retryWithoutStateOnRequestTooLarge(
90+
Raven.captureMessage = retryWithoutStateIfRequestTooLarge(
7691
Raven.captureMessage
7792
);
7893

index.test.js

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ const Raven = require("raven-js");
22
const createRavenMiddleware = require("./index");
33
const { createStore, applyMiddleware } = require("redux");
44

5+
const stringify = require.requireActual(
6+
"./vendor/json-stringify-safe/stringify"
7+
);
8+
const stringifyMocked = require("./vendor/json-stringify-safe/stringify");
9+
jest.mock("./vendor/json-stringify-safe/stringify");
10+
511
Raven.config(
612
"https://[email protected]/146969"
713
).install();
@@ -27,6 +33,7 @@ const context = {};
2733

2834
describe("raven-for-redux", () => {
2935
beforeEach(() => {
36+
stringifyMocked.mockImplementation(obj => stringify(obj));
3037
context.mockTransport = jest.fn();
3138
Raven.setTransport(context.mockTransport);
3239
Raven.setDataCallback(undefined);
@@ -188,13 +195,48 @@ describe("raven-for-redux", () => {
188195
});
189196

190197
["captureException", "captureMessage"].forEach(fnName => {
198+
it(`skips state for ${fnName} if state is larger than 200000B`, () => {
199+
expect(Raven._globalOptions.transport).toEqual(context.mockTransport);
200+
stringifyMocked
201+
.mockClear()
202+
.mockImplementationOnce(() => ({ length: 200001 }))
203+
.mockImplementationOnce(() => ({ length: 500 }));
204+
// Test that allowDuplicates is set to true inside our handler and reset afterwards
205+
// (Error message needs to be unique for each test, because we set allowDuplicates to null)
206+
Raven._globalOptions.allowDuplicates = null;
207+
Raven[fnName].call(
208+
Raven,
209+
fnName === "captureException"
210+
? new Error("Test skip state")
211+
: "Test skip state"
212+
);
213+
214+
// Ensure transport and allowDuplicates have been reset
215+
expect(Raven._globalOptions.transport).toEqual(context.mockTransport);
216+
expect(Raven._globalOptions.allowDuplicates).toEqual(null);
217+
expect(context.mockTransport).toHaveBeenCalledTimes(1);
218+
const { extra } = context.mockTransport.mock.calls[0][0].data;
219+
expect(extra).toMatchObject({
220+
state:
221+
"Could not send state because request would be larger than 200KB. (Was: 200001B)",
222+
lastAction: undefined
223+
});
224+
});
225+
191226
it(`retries ${fnName} without any state if Sentry returns 413 request too large`, () => {
227+
expect(Raven._globalOptions.transport).toEqual(context.mockTransport);
192228
context.mockTransport.mockImplementationOnce(options => {
193229
options.onError({ request: { status: 413 } });
194230
});
195-
// allowDuplicates is set to true inside our handler and reset afterwards
231+
// Test that allowDuplicates is set to true inside our handler and reset afterwards
232+
// (Error message needs to be unique for each test, because we set allowDuplicates to null)
196233
Raven._globalOptions.allowDuplicates = null;
197-
Raven[fnName].call(Raven, new Error("Crash!"));
234+
Raven[fnName].call(
235+
Raven,
236+
fnName === "captureException"
237+
? new Error("Test retry on 413 error")
238+
: "Test retry on 413 error"
239+
);
198240

199241
// Ensure transport and allowDuplicates have been reset
200242
expect(Raven._globalOptions.transport).toEqual(context.mockTransport);

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
"scripts": {
77
"fix": "eslint --fix .",
88
"tdd": "jest --watch",
9-
"test": "jest --coverage && npm run lint",
9+
"test": "jest --coverage --collectCoverageFrom=index.js && npm run lint",
1010
"lint": "eslint .",
11-
"build": "babel index.js --out-dir built/",
11+
"build": "babel index.js vendor/json-stringify-safe/stringify.js --out-dir built/",
1212
"prepublishOnly": "npm run test && npm run build",
1313
"serve-example": "webpack-dev-server --watch --config=example/webpack.config.js"
1414
},
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
json-stringify-safe
3+
Like JSON.stringify, but doesn't throw on circular references.
4+
5+
Copied from sentry-javascript/packages/raven-js/vendor/json-stringify-safe/stringify.js
6+
7+
Originally forked from https://github.com/isaacs/json-stringify-safe
8+
version 5.0.1 on 3/8/2017 and modified to handle Errors serialization
9+
and IE8 compatibility. Tests for this are in test/vendor.
10+
11+
ISC license: https://github.com/isaacs/json-stringify-safe/blob/master/LICENSE
12+
*/
13+
14+
exports = module.exports = stringify;
15+
exports.getSerialize = serializer;
16+
17+
function indexOf(haystack, needle) {
18+
for (var i = 0; i < haystack.length; ++i) {
19+
if (haystack[i] === needle) return i;
20+
}
21+
return -1;
22+
}
23+
24+
function stringify(obj, replacer, spaces, cycleReplacer) {
25+
return JSON.stringify(obj, serializer(replacer, cycleReplacer), spaces);
26+
}
27+
28+
// https://github.com/ftlabs/js-abbreviate/blob/fa709e5f139e7770a71827b1893f22418097fbda/index.js#L95-L106
29+
function stringifyError(value) {
30+
var err = {
31+
// These properties are implemented as magical getters and don't show up in for in
32+
stack: value.stack,
33+
message: value.message,
34+
name: value.name
35+
};
36+
37+
for (var i in value) {
38+
if (Object.prototype.hasOwnProperty.call(value, i)) {
39+
err[i] = value[i];
40+
}
41+
}
42+
43+
return err;
44+
}
45+
46+
function serializer(replacer, cycleReplacer) {
47+
var stack = [];
48+
var keys = [];
49+
50+
if (cycleReplacer == null) {
51+
cycleReplacer = function(key, value) {
52+
if (stack[0] === value) {
53+
return '[Circular ~]';
54+
}
55+
return '[Circular ~.' + keys.slice(0, indexOf(stack, value)).join('.') + ']';
56+
};
57+
}
58+
59+
return function(key, value) {
60+
if (stack.length > 0) {
61+
var thisPos = indexOf(stack, this);
62+
~thisPos ? stack.splice(thisPos + 1) : stack.push(this);
63+
~thisPos ? keys.splice(thisPos, Infinity, key) : keys.push(key);
64+
65+
if (~indexOf(stack, value)) {
66+
value = cycleReplacer.call(this, key, value);
67+
}
68+
} else {
69+
stack.push(value);
70+
}
71+
72+
return replacer == null
73+
? value instanceof Error ? stringifyError(value) : value
74+
: replacer.call(this, key, value);
75+
};
76+
}

0 commit comments

Comments
 (0)