From ba3c46db1488dd577099ac2671118d0d9af28f2e Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Thu, 7 Apr 2016 14:50:34 -0700 Subject: [PATCH] Fix XHR breadcrumbs not captured when no onreadystatechange handler set --- src/raven.js | 33 ++++++++++++++++++++++++--------- test/integration/test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/raven.js b/src/raven.js index 47dd264aa344..f9ba1cdfb103 100644 --- a/src/raven.js +++ b/src/raven.js @@ -742,21 +742,36 @@ Raven.prototype = { fill(xhrproto, 'send', function(origSend) { return function (data) { // preserve arity var xhr = this; - 'onreadystatechange onload onerror onprogress'.replace(/\w+/g, function (prop) { + + function onreadystatechangeHandler() { + if (xhr.__raven_xhr && (xhr.readyState === 1 || xhr.readyState === 4)) { + try { + // touching statusCode in some platforms throws + // an exception + xhr.__raven_xhr.status_code = xhr.status; + } catch (e) { /* do nothing */ } + self.captureBreadcrumb('http_request', xhr.__raven_xhr); + } + } + + 'onload onerror onprogress'.replace(/\w+/g, function (prop) { if (prop in xhr && isFunction(xhr[prop])) { fill(xhr, prop, function (orig) { - if (prop === 'onreadystatechange' && xhr.__raven_xhr && (xhr.readyState === 1 || xhr.readyState === 4)) { - try { - // touching statusCode in some platforms throws - // an exception - xhr.__raven_xhr.status_code = xhr.status; - } catch (e) { /* do nothing */ } - self.captureBreadcrumb('http_request', xhr.__raven_xhr); - } return self.wrap(orig); }, true /* noUndo */); // don't track filled methods on XHR instances } }); + + if ('onreadystatechange' in xhr && isFunction(xhr.onreadystatechange)) { + fill(xhr, 'onreadystatechange', function (orig) { + return self.wrap(orig, undefined, onreadystatechangeHandler); + }, true /* noUndo */); + } else { + // if onreadystatechange wasn't actually set by the page on this xhr, we + // are free to set our own and capture the breadcrumb + xhr.onreadystatechange = onreadystatechangeHandler; + } + return origSend.apply(this, arguments); }; }); diff --git a/test/integration/test.js b/test/integration/test.js index 3db63c199c47..2d4814645131 100644 --- a/test/integration/test.js +++ b/test/integration/test.js @@ -338,6 +338,40 @@ describe('integration', function () { ); }); + it('should record an XMLHttpRequest without any handlers set', function (done) { + var iframe = this.iframe; + + iframeExecute(iframe, done, + function () { + // I hate to do a time-based "done" trigger, but unfortunately we can't + // set an onload/onreadystatechange handler on XHR to verify that it finished + // - that's the whole point of this test! :( + setTimeout(done, 1000); + + // some browsers trigger onpopstate for load / reset breadcrumb state + Raven._breadcrumbs = []; + + var xhr = new XMLHttpRequest(); + + xhr.open('GET', '/test/integration/example.json'); + xhr.setRequestHeader('Content-type', 'application/json'); + xhr.send(); + }, + function () { + var Raven = iframe.contentWindow.Raven, + breadcrumbs = Raven._breadcrumbs; + + assert.equal(breadcrumbs.length, 1); + + assert.equal(breadcrumbs[0].type, 'http_request'); + assert.equal(breadcrumbs[0].data.method, 'GET'); + // NOTE: not checking status code because we seem to get + // statusCode 0/undefined from Phantom when fetching + // example.json (CORS issue? + } + ); + }); + it('should record a mouse click on element WITH click handler present', function (done) { var iframe = this.iframe;