Merge pull request #550 from getsentry/xhr-breadcrumb-bug

Fix XHR breadcrumbs not captured when no onreadystatechange handler set
getsentry · Apr 7, 2016 · d1050ce · d1050ce
2 parents 2e21555 + ba3c46d
commit d1050ce
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 9 deletions.
diff --git 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
@@ -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;