Skip to content

Commit 0784c3c

Browse files
committed
Merge pull request #574 from getsentry/full-nav-urls
Include full urls (query, doc fragment) in navigation breadcrumbs
2 parents 7ebc82f + 8678d95 commit 0784c3c

File tree

4 files changed

+29
-19
lines changed

4 files changed

+29
-19
lines changed

src/raven.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,9 @@ Raven.prototype = {
714714
// Use only the path component of the URL if the URL matches the current
715715
// document (almost all the time when using pushState)
716716
if (parsedLoc.protocol === parsedTo.protocol && parsedLoc.host === parsedTo.host)
717-
to = parsedTo.path;
717+
to = parsedTo.relative;
718718
if (parsedLoc.protocol === parsedFrom.protocol && parsedLoc.host === parsedFrom.host)
719-
from = parsedFrom.path;
719+
from = parsedFrom.relative;
720720

721721
this.captureBreadcrumb({
722722
category: 'navigation',

src/utils.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,15 @@ function urlencode(o) {
114114
function parseUrl(url) {
115115
var match = url.match(/^(([^:\/?#]+):)?(\/\/([^\/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?$/);
116116
if (!match) return {};
117+
118+
// coerce to undefined values to empty string so we don't get 'undefined'
119+
var query = match[6] || '';
120+
var fragment = match[8] || '';
117121
return {
118122
protocol: match[2],
119123
host: match[4],
120-
path: match[5]
124+
path: match[5],
125+
relative: match[5] + query + fragment // everything minus origin
121126
};
122127
}
123128
function uuid4() {

test/integration/test.js

+9-8
Original file line numberDiff line numberDiff line change
@@ -659,15 +659,16 @@ describe('integration', function () {
659659
function () {
660660
// some browsers trigger onpopstate for load / reset breadcrumb state
661661
Raven._breadcrumbs = [];
662+
662663
history.pushState({}, '', '/foo');
663-
history.pushState({}, '', '/bar');
664+
history.pushState({}, '', '/bar?a=1#fragment');
664665
history.pushState({}, '', {}); // pushState calls toString on non-string args
665666
history.pushState({}, '', null); // does nothing / no-op
666667

667668
// can't call history.back() because it will change url of parent document
668669
// (e.g. document running mocha) ... instead just "emulate" a back button
669670
// press by calling replaceState + onpopstate manually
670-
history.replaceState({}, '', '/bar');
671+
history.replaceState({}, '', '/bar?a=1#fragment');
671672
window.onpopstate();
672673
done();
673674
},
@@ -679,22 +680,22 @@ describe('integration', function () {
679680

680681
assert.equal(breadcrumbs.length, 4);
681682
assert.equal(breadcrumbs[0].category, 'navigation'); // (start) => foo
682-
assert.equal(breadcrumbs[1].category, 'navigation'); // foo => bar
683-
assert.equal(breadcrumbs[2].category, 'navigation'); // bar => [object%20Object]
684-
assert.equal(breadcrumbs[3].category, 'navigation'); // [object%20Object] => bar(back button)
683+
assert.equal(breadcrumbs[1].category, 'navigation'); // foo => bar?a=1#fragment
684+
assert.equal(breadcrumbs[2].category, 'navigation'); // bar?a=1#fragment => [object%20Object]
685+
assert.equal(breadcrumbs[3].category, 'navigation'); // [object%20Object] => bar?a=1#fragment (back button)
685686

686687
// assert end of string because PhantomJS uses full system path
687688
assert.ok(/\/test\/integration\/frame\.html$/.test(Raven._breadcrumbs[0].data.from), '\'from\' url is incorrect');
688689
assert.ok(/\/foo$/.test(breadcrumbs[0].data.to), '\'to\' url is incorrect');
689690

690691
assert.ok(/\/foo$/.test(breadcrumbs[1].data.from), '\'from\' url is incorrect');
691-
assert.ok(/\/bar$/.test(breadcrumbs[1].data.to), '\'to\' url is incorrect');
692+
assert.ok(/\/bar\?a=1#fragment$/.test(breadcrumbs[1].data.to), '\'to\' url is incorrect');
692693

693-
assert.ok(/\/bar$/.test(breadcrumbs[2].data.from), '\'from\' url is incorrect');
694+
assert.ok(/\/bar\?a=1#fragment$/.test(breadcrumbs[2].data.from), '\'from\' url is incorrect');
694695
assert.ok(/\[object Object\]$/.test(breadcrumbs[2].data.to), '\'to\' url is incorrect');
695696

696697
assert.ok(/\[object Object\]$/.test(breadcrumbs[3].data.from), '\'from\' url is incorrect');
697-
assert.ok(/\/bar/.test(breadcrumbs[3].data.to), '\'to\' url is incorrect');
698+
assert.ok(/\/bar\?a=1#fragment/.test(breadcrumbs[3].data.to), '\'to\' url is incorrect');
698699
}
699700
);
700701
});

test/utils.test.js

+12-8
Original file line numberDiff line numberDiff line change
@@ -232,27 +232,31 @@ describe('utils', function () {
232232
describe('parseUrl', function () {
233233
it('should parse fully qualified URLs', function () {
234234
assert.deepEqual(parseUrl('http://example.com/foo'), {
235+
protocol: 'http',
235236
host: 'example.com',
236237
path: '/foo',
237-
protocol: 'http'
238+
relative: '/foo'
238239
});
239-
assert.deepEqual(parseUrl('//example.com/foo'), {
240+
assert.deepEqual(parseUrl('//example.com/foo?query'), {
241+
protocol: undefined,
240242
host: 'example.com',
241243
path: '/foo',
242-
protocol: undefined
244+
relative: '/foo?query'
243245
});
244246
});
245247

246248
it('should parse partial URLs, e.g. path only', function () {
247249
assert.deepEqual(parseUrl('/foo'), {
248-
host: undefined,
249250
protocol: undefined,
250-
path: '/foo'
251-
});
252-
assert.deepEqual(parseUrl('example.com/foo'), {
253251
host: undefined,
252+
path: '/foo',
253+
relative: '/foo'
254+
});
255+
assert.deepEqual(parseUrl('example.com/foo#derp'), {
254256
protocol: undefined,
255-
path: 'example.com/foo'
257+
host: undefined,
258+
path: 'example.com/foo',
259+
relative: 'example.com/foo#derp'
256260
// this is correct! pushState({}, '', 'example.com/foo') would take you
257261
// from example.com => example.com/example.com/foo (valid url).
258262
});

0 commit comments

Comments
 (0)