Skip to content

Commit

Permalink
Fix that timer.addAction reverses order when splicing multiple events…
Browse files Browse the repository at this point in the history
… at same timestamp (#611)

* Add test for event ordering utilizing final html testing method added by Justin (with thanks)

* Found an error where two mutation events had the same timestamp, but one removed a node added in the other. The `actions.splice` method was reversing their order and triggering a 'Node with id [...] not found' error
  • Loading branch information
eoghanmurray authored Aug 9, 2021
1 parent 188f31e commit 009d73a
Show file tree
Hide file tree
Showing 4 changed files with 218 additions and 1 deletion.
4 changes: 3 additions & 1 deletion packages/rrweb/src/replay/timer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ export class Timer {
} else if (this.actions[mid].delay > action.delay) {
end = mid - 1;
} else {
return mid;
// already an action with same delay (timestamp)
// the plus one will splice the new one after the existing one
return mid + 1;
}
}
return start;
Expand Down
51 changes: 51 additions & 0 deletions packages/rrweb/test/__snapshots__/replayer.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,56 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ordering-events 1`] = `
"file-frame-1
<html>
<head>
<meta http-equiv=\\"Content-Type\\" content=\\"text/html; charset=UTF-8\\" />
</head>
<body>
<div class=\\"replayer-wrapper\\">
<div class=\\"replayer-mouse\\"></div>
<canvas
class=\\"replayer-mouse-tail\\"
width=\\"1000\\"
height=\\"800\\"
style=\\"display: inherit\\"
></canvas
><iframe
sandbox=\\"allow-same-origin\\"
scrolling=\\"no\\"
width=\\"1000\\"
height=\\"800\\"
style=\\"display: inherit; pointer-events: none\\"
></iframe>
</div>
</body>
</html>
file-frame-2
<!DOCTYPE html>
<html lang=\\"en\\">
<head>
<meta http-equiv=\\"Content-Type\\" content=\\"text/html; charset=UTF-8\\" />
<link rel=\\"stylesheet\\" type=\\"text/css\\" href=\\"file-cid-0\\" />
</head>
<body>
<span>Final - correct</span>
</body>
</html>
file-cid-0
@charset \\"utf-8\\";
.rr-block { background: rgb(204, 204, 204); }
noscript { display: none !important; }
html.rrweb-paused * { animation-play-state: paused !important; }
"
`;

exports[`style-sheet-remove-events-play-at-2500 1`] = `
"file-frame-1
<html>
Expand Down
123 changes: 123 additions & 0 deletions packages/rrweb/test/events/ordering.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { EventType, eventWithTime, IncrementalSource } from '../../src/types';

const now = Date.now();
const events: eventWithTime[] = [
{
type: EventType.DomContentLoaded,
data: {},
timestamp: now,
},
{
type: EventType.Load,
data: {},
timestamp: now + 10,
},
{
type: EventType.Meta,
data: {
href: 'http://localhost',
width: 1000,
height: 800,
},
timestamp: now + 10,
},
// full snapshot:
{
data: {
node: {
id: 1,
type: 0,
childNodes: [
{ id: 2, name: 'html', type: 1, publicId: '', systemId: '' },
{
id: 3,
type: 2,
tagName: 'html',
attributes: { lang: 'en' },
childNodes: [
{
id: 4,
type: 2,
tagName: 'head',
attributes: {},
childNodes: [],
},
{
id: 100,
type: 2,
tagName: 'body',
attributes: {},
childNodes: [
{
id: 101,
type: 2,
tagName: 'span',
attributes: {},
childNodes: [
{
id: 102,
type: 3,
textContent: 'Initial',
},
],
},
],
},
],
},
],
},
initialOffset: { top: 0, left: 0 },
},
type: EventType.FullSnapshot,
timestamp: now + 20,
},
// 1st mutation that modifies text content
{
data: {
adds: [],
texts: [
{
id: 102,
value: 'Intermediate - incorrect',
}
],
source: IncrementalSource.Mutation,
removes: [],
attributes: [],
},
type: EventType.IncrementalSnapshot,
timestamp: now + 30,
},
// 2nd mutation (with same timestamp) that modifies text content
{
data: {
adds: [],
texts: [
{
id: 102,
value: 'Final - correct',
}
],
source: IncrementalSource.Mutation,
removes: [],
attributes: [],
},
type: EventType.IncrementalSnapshot,
timestamp: now + 30,
},
// dummy - presence triggers a bug
{
data: {
adds: [],
texts: [],
source: IncrementalSource.Mutation,
removes: [],
attributes: [],
},
type: EventType.IncrementalSnapshot,
timestamp: now + 35,
},
];

export default events;
41 changes: 41 additions & 0 deletions packages/rrweb/test/replayer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
sampleStyleSheetRemoveEvents as stylesheetRemoveEvents,
} from './utils';
import styleSheetRuleEvents from './events/style-sheet-rule-events';
import orderingEvents from './events/ordering';

interface ISuite extends Suite {
code: string;
Expand Down Expand Up @@ -247,4 +248,44 @@ describe('replayer', function (this: ISuite) {
`);
expect(status).to.equal('live');
});

it('replays same timestamp events in correct order', async () => {
await this.page.evaluate(
`events = ${JSON.stringify(orderingEvents)}`,
);
await this.page.evaluate(`
const { Replayer } = rrweb;
const replayer = new Replayer(events);
replayer.play();
`);
await this.page.waitForTimeout(50);

await assertDomSnapshot(
this.page,
__filename,
'ordering-events',
);
});

it('replays same timestamp events in correct order (with addAction)', async () => {
await this.page.evaluate(
`events = ${JSON.stringify(orderingEvents)}`,
);
await this.page.evaluate(`
const { Replayer } = rrweb;
const replayer = new Replayer(events.slice(0, events.length-2));
replayer.play();
replayer.addEvent(events[events.length-2]);
replayer.addEvent(events[events.length-1]);
`);
await this.page.waitForTimeout(50);

await assertDomSnapshot(
this.page,
__filename,
'ordering-events',
);
});


});

0 comments on commit 009d73a

Please sign in to comment.