From 009d73ab79cbc3ac66bb255dee9065dc6b1d84ec Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Mon, 9 Aug 2021 17:03:24 +0100 Subject: [PATCH] Fix that timer.addAction reverses order when splicing multiple events 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 --- packages/rrweb/src/replay/timer.ts | 4 +- .../test/__snapshots__/replayer.test.ts.snap | 51 ++++++++ packages/rrweb/test/events/ordering.ts | 123 ++++++++++++++++++ packages/rrweb/test/replayer.test.ts | 41 ++++++ 4 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 packages/rrweb/test/events/ordering.ts diff --git a/packages/rrweb/src/replay/timer.ts b/packages/rrweb/src/replay/timer.ts index 2d2d7c23..684c3ff0 100644 --- a/packages/rrweb/src/replay/timer.ts +++ b/packages/rrweb/src/replay/timer.ts @@ -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; diff --git a/packages/rrweb/test/__snapshots__/replayer.test.ts.snap b/packages/rrweb/test/__snapshots__/replayer.test.ts.snap index de5db0c3..c95a251b 100644 --- a/packages/rrweb/test/__snapshots__/replayer.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/replayer.test.ts.snap @@ -1,5 +1,56 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`ordering-events 1`] = ` +"file-frame-1 + + + + + +
+
+ +
+ + + + +file-frame-2 + + + + + + + + Final - correct + + + + +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 diff --git a/packages/rrweb/test/events/ordering.ts b/packages/rrweb/test/events/ordering.ts new file mode 100644 index 00000000..083c6870 --- /dev/null +++ b/packages/rrweb/test/events/ordering.ts @@ -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; diff --git a/packages/rrweb/test/replayer.test.ts b/packages/rrweb/test/replayer.test.ts index bc37e32f..4ffd9b9f 100644 --- a/packages/rrweb/test/replayer.test.ts +++ b/packages/rrweb/test/replayer.test.ts @@ -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; @@ -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', + ); + }); + + });