Make sure we finish sending the HTML payload before we exit the process (#38)

I encountered a page which responded successfully but all of the Hydrogen HTML was missing. It just had the boilerplate around it.

What I am guessing happened is that since `process.send` is async, with a sufficiently large
payload and race condition, `process.exit(0)` was being called before it finished sending.

Related:
 - https://stackoverflow.com/questions/34627546/process-send-is-sync-async-on-nix-windows
 - 56d9584a0e
 - https://github.com/nodejs/node/issues/6767
This commit is contained in:
Eric Eastwood 2022-07-06 19:24:29 -05:00 committed by GitHub
parent 7eaa103a28
commit 13eb92b067
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 7 deletions

View File

@ -6,6 +6,7 @@
const fork = require('child_process').fork; const fork = require('child_process').fork;
const assert = require('assert');
const RethrownError = require('../lib/rethrown-error'); const RethrownError = require('../lib/rethrown-error');
// The render should be fast. If it's taking more than 5 seconds, something has // The render should be fast. If it's taking more than 5 seconds, something has
@ -60,12 +61,16 @@ async function renderHydrogenToString(options) {
} else { } else {
let extraErrorsMessage = ''; let extraErrorsMessage = '';
if (childErrors.length > 1) { if (childErrors.length > 1) {
extraErrorsMessage = ` (somehow we saw ${childErrors.length} errors but we really always expect 1 error)`; extraErrorsMessage = ` (somehow we saw ${
childErrors.length
} errors but we really always expect 1 error)\n${childErrors
.map((childError, index) => ` ${index}. ${childError.message} ${childError.stack}`)
.join('\n')}`;
} }
const error = new RethrownError( const error = new RethrownError(
`Child process failed with exit code ${exitCode}${extraErrorsMessage}`, `Child process failed with exit code ${exitCode}${extraErrorsMessage}`,
childErrors[0] childErrors[0] || new Error('No child errors')
); );
reject(error); reject(error);
} }
@ -84,6 +89,11 @@ async function renderHydrogenToString(options) {
}); });
}); });
assert(
data,
`No HTML sent from child process to render Hydrogen. Any child errors? (${childErrors.length})`
);
return data; return data;
} catch (err) { } catch (err) {
throw new RethrownError( throw new RethrownError(

View File

@ -4,6 +4,8 @@
// get the data and exit the process cleanly. We don't want Hydrogen to keep // get the data and exit the process cleanly. We don't want Hydrogen to keep
// running after we get our initial rendered HTML. // running after we get our initial rendered HTML.
const assert = require('assert');
const _renderHydrogenToStringUnsafe = require('./3-render-hydrogen-to-string-unsafe'); const _renderHydrogenToStringUnsafe = require('./3-render-hydrogen-to-string-unsafe');
// Only kick everything off once we receive the options. We pass in the options // Only kick everything off once we receive the options. We pass in the options
@ -13,12 +15,31 @@ process.on('message', async (options) => {
try { try {
const resultantHtml = await _renderHydrogenToStringUnsafe(options); const resultantHtml = await _renderHydrogenToStringUnsafe(options);
// Send back the data we need assert(resultantHtml, `No HTML returned from _renderHydrogenToStringUnsafe.`);
process.send({
// Send back the data we need to the parent.
await new Promise((resolve, reject) => {
process.send(
{
data: resultantHtml, data: resultantHtml,
}); },
// End the process gracefully. We got all the data we need. (err) => {
if (err) {
return reject(err);
}
// Exit once we know the data was sent out. We can't gurantee the
// message was received but this should work pretty well.
//
// Related:
// - https://stackoverflow.com/questions/34627546/process-send-is-sync-async-on-nix-windows
// - https://github.com/nodejs/node/commit/56d9584a0ead78874ca9d4de2e55b41c4056e502
// - https://github.com/nodejs/node/issues/6767
process.exit(0); process.exit(0);
resolve();
}
);
});
} catch (err) { } catch (err) {
// Serialize the error and send it back up to the parent process so we can // Serialize the error and send it back up to the parent process so we can
// interact with it and know what happened when the process exits. // interact with it and know what happened when the process exits.

View File

@ -94,6 +94,7 @@ async function _renderHydrogenToStringUnsafe({ fromTimestamp, roomData, events,
await vmResult; await vmResult;
const documentString = dom.document.body.toString(); const documentString = dom.document.body.toString();
assert(documentString, 'Document body should not be empty after we rendered Hydrogen');
return documentString; return documentString;
} }