Skip to content

Commit

Permalink
Merge pull request #843 from rollbar/wj-http-error-stack
Browse files Browse the repository at this point in the history
Only build stack for http errors when the feature is enabled
  • Loading branch information
waltjones authored Apr 14, 2020
2 parents 3792739 + 8d1312c commit 802b5ce
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/browser/telemetry.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Instrumenter.prototype.instrumentNetwork = function() {
} else {
xhr.onreadystatechange = onreadystatechangeHandler;
}
if (xhr.__rollbar_xhr) {
if (xhr.__rollbar_xhr && self.trackHttpErrors()) {
xhr.__rollbar_xhr.stack = (new Error()).stack;
}
return orig.apply(this, arguments);
Expand Down Expand Up @@ -354,7 +354,9 @@ Instrumenter.prototype.instrumentNetwork = function() {
}
}
self.captureNetwork(metadata, 'fetch', undefined);
metadata.stack = (new Error()).stack;
if (self.trackHttpErrors()) {
metadata.stack = (new Error()).stack;
}
return orig.apply(this, args).then(function (resp) {
metadata.end_time_ms = _.now();
metadata.status_code = resp.status;
Expand Down Expand Up @@ -435,6 +437,12 @@ Instrumenter.prototype.fetchHeaders = function(inHeaders, headersConfig) {
return outHeaders;
}

Instrumenter.prototype.trackHttpErrors = function() {
return this.autoInstrument.networkErrorOnHttp5xx ||
this.autoInstrument.networkErrorOnHttp4xx ||
this.autoInstrument.networkErrorOnHttp0;
}

Instrumenter.prototype.errorOnHttpStatus = function(metadata) {
var status = metadata.status_code;

Expand Down
99 changes: 99 additions & 0 deletions test/browser.rollbar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,54 @@ describe('options.autoInstrument', function() {
server.respond();
});

it('should send errors for xhr http errors', function(done) {
var server = window.server;
stubResponse(server);
server.requests.length = 0;

server.respondWith('POST', 'xhr-test',
[
404,
{ 'Content-Type': 'application/json' },
JSON.stringify({foo: 'bar'})
]
);

var options = {
accessToken: 'POST_CLIENT_ITEM_TOKEN',
autoInstrument: {
log: false,
network: true,
networkErrorOnHttp4xx: true
}
};
window.rollbar = new Rollbar(options);

// generate a telemetry event
var xhr = new XMLHttpRequest();
xhr.open('POST', 'https://example.com/xhr-test', true);
xhr.setRequestHeader('Content-type', 'application/json');
xhr.onreadystatechange = function () {
if(xhr.readyState === 4) {
try {
expect(server.requests.length).to.eql(2);
var body = JSON.parse(server.requests[1].requestBody);

expect(body.data.body.trace.exception.message).to.eql('HTTP request failed with Status 404');

// Just knowing a stack is present is enough for this test.
expect(body.data.body.trace.frames.length).to.be.above(1);

done();
} catch (e) {
done(e);
}
}
};
xhr.send(JSON.stringify({name: 'bar', secret: 'xhr post' }));
server.respond();
});

it('should add telemetry events for fetch calls', function(done) {
var server = window.server;
stubResponse(server);
Expand Down Expand Up @@ -884,6 +932,57 @@ describe('options.autoInstrument', function() {
})
});

it('should add telemetry events for fetch calls', function(done) {
var server = window.server;
stubResponse(server);
server.requests.length = 0;

window.fetchStub = sinon.stub(window, 'fetch');
window.fetch.returns(Promise.resolve(new Response(
JSON.stringify({foo: 'bar'}),
{ status: 404, statusText: 'Not Found', headers: { 'content-type': 'application/json' }}
)));

var options = {
accessToken: 'POST_CLIENT_ITEM_TOKEN',
autoInstrument: {
log: false,
network: true,
networkErrorOnHttp4xx: true
}
};
window.rollbar = new Rollbar(options);

var fetchHeaders = new Headers();
fetchHeaders.append('Content-Type', 'application/json');

const fetchInit = {
method: 'POST',
headers: fetchHeaders,
body: JSON.stringify({foo: 'bar'})
};
var fetchRequest = new Request('https://example.com/xhr-test');
window.fetch(fetchRequest, fetchInit).then(function(_response) {
try {
server.respond();

expect(server.requests.length).to.eql(2);
var body = JSON.parse(server.requests[1].requestBody);

expect(body.data.body.trace.exception.message).to.eql('HTTP request failed with Status 404');

// Just knowing a stack is present is enough for this test.
expect(body.data.body.trace.frames.length).to.be.above(1);

rollbar.configure({ autoInstrument: false });
window.fetch.restore();
done();
} catch (e) {
done(e);
}
})
});

it('should add a diagnostic message when wrapConsole fails', function(done) {
var server = window.server;
stubResponse(server);
Expand Down

0 comments on commit 802b5ce

Please sign in to comment.