From 36925cd603ad288a3c78f86126a1d3bb4c0bfc0b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 29 Aug 2022 19:42:18 -0500 Subject: [PATCH] Add test to make sure the archive doesn't fail when event for event relation is missing and not included in list of provided events (#43) Add test to make sure the archive doesn't fail when event for event relation is missing and not included in list of provided events. Like if someone is replying to an event that was from long ago out of our range. In the case of missing relations, Hydrogen does `_loadContextEntryNotInTimeline` because it can't find the event locally which throws an `uncaughtException`. Before https://github.com/matrix-org/matrix-public-archive/pull/51, the `uncaughtException` killed the Hydrogen `child_process` before it could pass back the HTML. Now this PR mainly just adds a test to make sure it works. ``` TypeError: Cannot read properties of undefined (reading 'storeNames') at TimelineReader.readById (hydrogen-web\target\lib-build\hydrogen.cjs.js:12483:33) at Timeline._getEventFromStorage (hydrogen-web\target\lib-build\hydrogen.cjs.js:12762:46) at Timeline._loadContextEntryNotInTimeline (hydrogen-web\target\lib-build\hydrogen.cjs.js:12747:35) at Timeline._loadContextEntriesWhereNeeded (hydrogen-web\target\lib-build\hydrogen.cjs.js:12741:14) at Timeline.addEntries (hydrogen-web\target\lib-build\hydrogen.cjs.js:12699:10) at mountHydrogen (4-hydrogen-vm-render-script.js:204:12) at 4-hydrogen-vm-render-script.js:353:1 at Script.runInContext (node:vm:139:12) at _renderHydrogenToStringUnsafe (matrix-public-archive\server\hydrogen-render\3-render-hydrogen-to-string-unsafe.js:102:41) at async process. (matrix-public-archive\server\hydrogen-render\2-render-hydrogen-to-string-fork-script.js:18:27) ``` --- .../3-render-hydrogen-to-string-unsafe.js | 56 +++++++++++-------- test/client-utils.js | 1 - test/e2e-tests.js | 42 ++++++++++++++ 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/server/hydrogen-render/3-render-hydrogen-to-string-unsafe.js b/server/hydrogen-render/3-render-hydrogen-to-string-unsafe.js index a8bc83b..ec8c280 100644 --- a/server/hydrogen-render/3-render-hydrogen-to-string-unsafe.js +++ b/server/hydrogen-render/3-render-hydrogen-to-string-unsafe.js @@ -20,12 +20,10 @@ const { parseHTML } = require('linkedom'); const config = require('../lib/config'); -async function _renderHydrogenToStringUnsafe({ fromTimestamp, roomData, events, stateEventMap }) { - assert(fromTimestamp); - assert(roomData); - assert(events); - assert(stateEventMap); - +// Setup the DOM context with any necessary shims/polyfills and ensure the VM +// context global has everything that a normal document does so Hydrogen can +// render. +function createDomAndSetupVmContext() { const dom = parseHTML(` @@ -42,6 +40,36 @@ async function _renderHydrogenToStringUnsafe({ fromTimestamp, roomData, events, }; } + const vmContext = vm.createContext(dom); + // Make the dom properties available in sub-`require(...)` calls + vmContext.global.window = dom.window; + vmContext.global.document = dom.document; + vmContext.global.Node = dom.Node; + vmContext.global.navigator = dom.navigator; + vmContext.global.DOMParser = dom.DOMParser; + // Make sure `webcrypto` exists since it was only introduced in Node.js v17 + assert(crypto.webcrypto); + vmContext.global.crypto = crypto.webcrypto; + + // So require(...) works in the vm + vmContext.global.require = require; + // So we can see logs from the underlying vm + vmContext.global.console = console; + + return { + dom, + vmContext, + }; +} + +async function _renderHydrogenToStringUnsafe({ fromTimestamp, roomData, events, stateEventMap }) { + assert(fromTimestamp); + assert(roomData); + assert(events); + assert(stateEventMap); + + const { dom, vmContext } = createDomAndSetupVmContext(); + // Define this for the SSR context dom.window.matrixPublicArchiveContext = { fromTimestamp, @@ -64,22 +92,6 @@ async function _renderHydrogenToStringUnsafe({ fromTimestamp, roomData, events, ` ); - const vmContext = vm.createContext(dom); - // Make the dom properties available in sub-`require(...)` calls - vmContext.global.window = dom.window; - vmContext.global.document = dom.document; - vmContext.global.Node = dom.Node; - vmContext.global.navigator = dom.navigator; - vmContext.global.DOMParser = dom.DOMParser; - // Make sure `webcrypto` exists since it was only introduced in Node.js v17 - assert(crypto.webcrypto); - vmContext.global.crypto = crypto.webcrypto; - - // So require(...) works in the vm - vmContext.global.require = require; - // So we can see logs from the underlying vm - vmContext.global.console = console; - const hydrogenRenderScriptCode = await readFile( path.resolve(__dirname, '../../shared/4-hydrogen-vm-render-script.js'), 'utf8' diff --git a/test/client-utils.js b/test/client-utils.js index a119aa4..edb42cb 100644 --- a/test/client-utils.js +++ b/test/client-utils.js @@ -121,7 +121,6 @@ async function joinRoom({ client, roomId, viaServers }) { client.homeserverUrl, `/_matrix/client/v3/join/${roomId}?${qs.toString()}` ); - console.log('test client joinRoomUrl', joinRoomUrl); const joinRoomResponse = await fetchEndpointAsJson(joinRoomUrl, { method: 'POST', accessToken: client.accessToken, diff --git a/test/e2e-tests.js b/test/e2e-tests.js index c221160..193c8fc 100644 --- a/test/e2e-tests.js +++ b/test/e2e-tests.js @@ -129,6 +129,7 @@ describe('matrix-public-archive', () => { let numMessagesSent = 0; afterEach(() => { if (interactive) { + // eslint-disable-next-line no-console console.log('Interactive URL for test', archiveUrl); } @@ -317,6 +318,36 @@ describe('matrix-public-archive', () => { }, }); + // Test to make sure we can render the page when the reply is missing the + // event it's replying to (the relation). + const replyMissingRelationMessageText = `While the giant-impact theory explains many lines of evidence, some questions are still unresolved, most of which involve the Moon's composition.`; + const missingRelationEventId = '$someMissingEvent'; + const replyMissingRelationMessageEventId = await sendMessageOnArchiveDate({ + client, + roomId, + content: { + 'org.matrix.msc1767.message': [ + { + body: '> <@ericgittertester:my.synapse.server> some missing message', + mimetype: 'text/plain', + }, + { + body: `
In reply to @ericgittertester:my.synapse.server
some missing message
${replyMissingRelationMessageText}`, + mimetype: 'text/html', + }, + ], + body: `> <@ericgittertester:my.synapse.server> some missing message\n\n${replyMissingRelationMessageText}`, + msgtype: 'm.text', + format: 'org.matrix.custom.html', + formatted_body: `
In reply to @ericgittertester:my.synapse.server
some missing message
${replyMissingRelationMessageText}`, + 'm.relates_to': { + 'm.in_reply_to': { + event_id: missingRelationEventId, + }, + }, + }, + }); + // Test reactions const reactionText = '😅'; await sendEventOnArchiveDate({ @@ -384,6 +415,17 @@ describe('matrix-public-archive', () => { replyMessageElement.outerHTML, new RegExp(`.*${escapeStringRegexp(normalMessageEventId2)}.*`) ); + + const replyMissingRelationMessageElement = dom.document.querySelector( + `[data-event-id="${replyMissingRelationMessageEventId}"]` + ); + // Make sure the reply text is there. + // We don't care about the message we're replying to because it's missing on purpose. + assert.match( + replyMissingRelationMessageElement.outerHTML, + new RegExp(`.*${escapeStringRegexp(replyMissingRelationMessageText)}.*`) + ); + // Make sure the reaction also exists assert.match( replyMessageElement.outerHTML,