From f03c4bd7f7c5108284167943f26a209d87daa2e4 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 8 Jan 2021 17:47:38 -0500 Subject: [PATCH 01/18] bin scripts: compare against null, not undefined --- bin/checkAllPads.js | 4 ++-- bin/checkPad.js | 8 ++++---- bin/checkPadDeltas.js | 4 +--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/bin/checkAllPads.js b/bin/checkAllPads.js index 6e1f14842..8426d4429 100644 --- a/bin/checkAllPads.js +++ b/bin/checkAllPads.js @@ -30,7 +30,7 @@ npm.load({}, async () => { const pad = await padManager.getPad(padId); // check if the pad has a pool - if (pad.pool === undefined) { + if (pad.pool == null) { console.error(`[${pad.id}] Missing attribute pool`); continue; } @@ -66,7 +66,7 @@ npm.load({}, async () => { } // check if there is a atext in the keyRevisions - if (revisions[keyRev].meta === undefined || revisions[keyRev].meta.atext === undefined) { + if (revisions[keyRev].meta == null || revisions[keyRev].meta.atext == null) { console.error(`[${pad.id}] Missing atext in revision ${keyRev}`); continue; } diff --git a/bin/checkPad.js b/bin/checkPad.js index de1c51402..374a3c856 100644 --- a/bin/checkPad.js +++ b/bin/checkPad.js @@ -59,12 +59,12 @@ npm.load({}, async () => { } // check if the pad has a pool - if (pad.pool === undefined) throw new Error('Attribute pool is missing'); + if (pad.pool == null) throw new Error('Attribute pool is missing'); // check if there is an atext in the keyRevisions - if (revisions[keyRev] === undefined || - revisions[keyRev].meta === undefined || - revisions[keyRev].meta.atext === undefined) { + if (revisions[keyRev] == null || + revisions[keyRev].meta == null || + revisions[keyRev].meta.atext == null) { console.error(`No atext in key revision ${keyRev}`); continue; } diff --git a/bin/checkPadDeltas.js b/bin/checkPadDeltas.js index ecbb20846..c53275a28 100644 --- a/bin/checkPadDeltas.js +++ b/bin/checkPadDeltas.js @@ -55,9 +55,7 @@ npm.load({}, async () => { const revision = await db.get(`pad:${padId}:revs:${revNum}`); // check if there is a atext in the keyRevisions if (~keyRevisions.indexOf(revNum) && - (revision === undefined || - revision.meta === undefined || - revision.meta.atext === undefined)) { + (revision == null || revision.meta == null || revision.meta.atext == null)) { console.error(`No atext in key revision ${revNum}`); continue; } From 92cd2cc760f369b14f48f0f3c3cf4feccf9568b1 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 9 Jan 2021 02:28:45 -0500 Subject: [PATCH 02/18] bin scripts: Use destructuring instead of long condition checks --- bin/checkAllPads.js | 4 ++-- bin/checkPad.js | 6 ++---- bin/checkPadDeltas.js | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/bin/checkAllPads.js b/bin/checkAllPads.js index 8426d4429..063da96a5 100644 --- a/bin/checkAllPads.js +++ b/bin/checkAllPads.js @@ -66,13 +66,13 @@ npm.load({}, async () => { } // check if there is a atext in the keyRevisions - if (revisions[keyRev].meta == null || revisions[keyRev].meta.atext == null) { + let {meta: {atext} = {}} = revisions[keyRev]; + if (atext == null) { console.error(`[${pad.id}] Missing atext in revision ${keyRev}`); continue; } const apool = pad.pool; - let atext = revisions[keyRev].meta.atext; for (let rev = keyRev + 1; rev <= keyRev + 100 && rev <= head; rev++) { try { const cs = revisions[rev].changeset; diff --git a/bin/checkPad.js b/bin/checkPad.js index 374a3c856..42e5946c1 100644 --- a/bin/checkPad.js +++ b/bin/checkPad.js @@ -62,15 +62,13 @@ npm.load({}, async () => { if (pad.pool == null) throw new Error('Attribute pool is missing'); // check if there is an atext in the keyRevisions - if (revisions[keyRev] == null || - revisions[keyRev].meta == null || - revisions[keyRev].meta.atext == null) { + let {meta: {atext} = {}} = revisions[keyRev] || {}; + if (atext == null) { console.error(`No atext in key revision ${keyRev}`); continue; } const apool = pad.pool; - let atext = revisions[keyRev].meta.atext; for (let rev = keyRev + 1; rev <= keyRev + 100 && rev <= head; rev++) { checkRevisionCount++; diff --git a/bin/checkPadDeltas.js b/bin/checkPadDeltas.js index c53275a28..19d4ba4db 100644 --- a/bin/checkPadDeltas.js +++ b/bin/checkPadDeltas.js @@ -54,8 +54,8 @@ npm.load({}, async () => { // console.log('Fetching', revNum) const revision = await db.get(`pad:${padId}:revs:${revNum}`); // check if there is a atext in the keyRevisions - if (~keyRevisions.indexOf(revNum) && - (revision == null || revision.meta == null || revision.meta.atext == null)) { + const {meta: {atext: revAtext} = {}} = revision || {}; + if (~keyRevisions.indexOf(revNum) && revAtext == null) { console.error(`No atext in key revision ${revNum}`); continue; } From efdcaae5269450ef49fdd6efb6fce9cc5fee2a6f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 9 Jan 2021 02:44:59 -0500 Subject: [PATCH 03/18] bin scripts: Promisify npm.load --- bin/checkAllPads.js | 11 +++++++---- bin/checkPad.js | 11 +++++++---- bin/checkPadDeltas.js | 8 +++++--- bin/extractPadData.js | 8 ++++---- bin/importSqlFile.js | 9 ++++++--- bin/migrateDirtyDBtoRealDB.js | 7 +++++-- bin/repairPad.js | 8 +++++--- 7 files changed, 39 insertions(+), 23 deletions(-) diff --git a/bin/checkAllPads.js b/bin/checkAllPads.js index 063da96a5..3e2ea8407 100644 --- a/bin/checkAllPads.js +++ b/bin/checkAllPads.js @@ -7,11 +7,14 @@ // unhandled rejection into an uncaught exception, which does cause Node.js to exit. process.on('unhandledRejection', (err) => { throw err; }); +const npm = require('ep_etherpad-lite/node_modules/npm'); +const util = require('util'); + if (process.argv.length !== 2) throw new Error('Use: node bin/checkAllPads.js'); -// load and initialize NPM -const npm = require('ep_etherpad-lite/node_modules/npm'); -npm.load({}, async () => { +(async () => { + await util.promisify(npm.load)({}); + try { // initialize the database require('ep_etherpad-lite/node/utils/Settings'); @@ -92,4 +95,4 @@ npm.load({}, async () => { console.trace(err); throw err; } -}); +})(); diff --git a/bin/checkPad.js b/bin/checkPad.js index 42e5946c1..56464648b 100644 --- a/bin/checkPad.js +++ b/bin/checkPad.js @@ -7,15 +7,18 @@ // unhandled rejection into an uncaught exception, which does cause Node.js to exit. process.on('unhandledRejection', (err) => { throw err; }); +const npm = require('ep_etherpad-lite/node_modules/npm'); +const util = require('util'); + if (process.argv.length !== 3) throw new Error('Use: node bin/checkPad.js $PADID'); // get the padID const padId = process.argv[2]; let checkRevisionCount = 0; -// load and initialize NPM; -const npm = require('ep_etherpad-lite/node_modules/npm'); -npm.load({}, async () => { +(async () => { + await util.promisify(npm.load)({}); + try { // initialize database require('ep_etherpad-lite/node/utils/Settings'); @@ -86,4 +89,4 @@ npm.load({}, async () => { console.trace(err); throw err; } -}); +})(); diff --git a/bin/checkPadDeltas.js b/bin/checkPadDeltas.js index 19d4ba4db..45ee27d72 100644 --- a/bin/checkPadDeltas.js +++ b/bin/checkPadDeltas.js @@ -12,12 +12,14 @@ if (process.argv.length !== 3) throw new Error('Use: node bin/checkPadDeltas.js // get the padID const padId = process.argv[2]; -// load and initialize NPM; const expect = require('../tests/frontend/lib/expect'); const diff = require('ep_etherpad-lite/node_modules/diff'); const npm = require('ep_etherpad-lite/node_modules/npm'); +const util = require('util'); + +(async () => { + await util.promisify(npm.load)({}); -npm.load({}, async () => { // initialize database require('ep_etherpad-lite/node/utils/Settings'); const db = require('ep_etherpad-lite/node/db/DB'); @@ -102,4 +104,4 @@ npm.load({}, async () => { } })); } -}); +})(); diff --git a/bin/extractPadData.js b/bin/extractPadData.js index 3b182a571..58fe50a42 100644 --- a/bin/extractPadData.js +++ b/bin/extractPadData.js @@ -16,9 +16,10 @@ if (process.argv.length !== 3) throw new Error('Use: node extractPadData.js $PAD const padId = process.argv[2]; const npm = require('ep_etherpad-lite/node_modules/npm'); +const util = require('util'); -npm.load({}, async (err) => { - if (err) throw err; +(async () => { + await util.promisify(npm.load)({}); try { // initialize database @@ -29,7 +30,6 @@ npm.load({}, async (err) => { // load extra modules const dirtyDB = require('ep_etherpad-lite/node_modules/dirty'); const padManager = require('ep_etherpad-lite/node/db/PadManager'); - const util = require('util'); // initialize output database const dirty = dirtyDB(`${padId}.db`); @@ -71,4 +71,4 @@ npm.load({}, async (err) => { console.error(err); throw err; } -}); +})(); diff --git a/bin/importSqlFile.js b/bin/importSqlFile.js index 35fe4f323..b02e9e10c 100644 --- a/bin/importSqlFile.js +++ b/bin/importSqlFile.js @@ -4,6 +4,9 @@ // unhandled rejection into an uncaught exception, which does cause Node.js to exit. process.on('unhandledRejection', (err) => { throw err; }); +const npm = require('ep_etherpad-lite/node_modules/npm'); +const util = require('util'); + const startTime = Date.now(); const log = (str) => { @@ -43,10 +46,10 @@ const unescape = (val) => { return val; }; +(async () => { + await util.promisify(npm.load)({}); -require('ep_etherpad-lite/node_modules/npm').load({}, (er, npm) => { const fs = require('fs'); - const ueberDB = require('ep_etherpad-lite/node_modules/ueberdb2'); const settings = require('ep_etherpad-lite/node/utils/Settings'); const log4js = require('ep_etherpad-lite/node_modules/log4js'); @@ -106,4 +109,4 @@ require('ep_etherpad-lite/node_modules/npm').load({}, (er, npm) => { }); } }); -}); +})(); diff --git a/bin/migrateDirtyDBtoRealDB.js b/bin/migrateDirtyDBtoRealDB.js index 48760b8ba..c79d9b747 100644 --- a/bin/migrateDirtyDBtoRealDB.js +++ b/bin/migrateDirtyDBtoRealDB.js @@ -4,9 +4,12 @@ // unhandled rejection into an uncaught exception, which does cause Node.js to exit. process.on('unhandledRejection', (err) => { throw err; }); +const npm = require('ep_etherpad-lite/node_modules/npm'); const util = require('util'); -require('ep_etherpad-lite/node_modules/npm').load({}, async (er, npm) => { +(async () => { + await util.promisify(npm.load)({}); + process.chdir(`${npm.root}/..`); // This script requires that you have modified your settings.json file @@ -56,4 +59,4 @@ require('ep_etherpad-lite/node_modules/npm').load({}, async (er, npm) => { await util.promisify(db.close.bind(db))(); console.log('Finished.'); -}); +})(); diff --git a/bin/repairPad.js b/bin/repairPad.js index e083f30b9..ff2da9776 100644 --- a/bin/repairPad.js +++ b/bin/repairPad.js @@ -18,8 +18,10 @@ const padId = process.argv[2]; let valueCount = 0; const npm = require('ep_etherpad-lite/node_modules/npm'); -npm.load({}, async (err) => { - if (err) throw err; +const util = require('util'); + +(async () => { + await util.promisify(npm.load)({}); // intialize database require('ep_etherpad-lite/node/utils/Settings'); @@ -56,4 +58,4 @@ npm.load({}, async (err) => { } console.info(`Finished: Replaced ${valueCount} values in the database`); -}); +})(); From 0a617679012728d7e48d28658cd08341d77808f6 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 17 Jan 2021 03:16:01 -0500 Subject: [PATCH 04/18] bin scripts: Delete redundant exception log messages The exception will cause Node.js to print the error message and stack trace so there's no point in logging it ourselves. --- bin/checkAllPads.js | 131 ++++++++++++++++++++---------------------- bin/checkPad.js | 107 ++++++++++++++++------------------ bin/extractPadData.js | 79 ++++++++++++------------- 3 files changed, 151 insertions(+), 166 deletions(-) diff --git a/bin/checkAllPads.js b/bin/checkAllPads.js index 3e2ea8407..356112e59 100644 --- a/bin/checkAllPads.js +++ b/bin/checkAllPads.js @@ -15,84 +15,79 @@ if (process.argv.length !== 2) throw new Error('Use: node bin/checkAllPads.js'); (async () => { await util.promisify(npm.load)({}); - try { - // initialize the database - require('ep_etherpad-lite/node/utils/Settings'); - const db = require('ep_etherpad-lite/node/db/DB'); - await db.init(); + // initialize the database + require('ep_etherpad-lite/node/utils/Settings'); + const db = require('ep_etherpad-lite/node/db/DB'); + await db.init(); - // load modules - const Changeset = require('ep_etherpad-lite/static/js/Changeset'); - const padManager = require('ep_etherpad-lite/node/db/PadManager'); + // load modules + const Changeset = require('ep_etherpad-lite/static/js/Changeset'); + const padManager = require('ep_etherpad-lite/node/db/PadManager'); - let revTestedCount = 0; + let revTestedCount = 0; - // get all pads - const res = await padManager.listAllPads(); - for (const padId of res.padIDs) { - const pad = await padManager.getPad(padId); + // get all pads + const res = await padManager.listAllPads(); + for (const padId of res.padIDs) { + const pad = await padManager.getPad(padId); - // check if the pad has a pool - if (pad.pool == null) { - console.error(`[${pad.id}] Missing attribute pool`); + // check if the pad has a pool + if (pad.pool == null) { + console.error(`[${pad.id}] Missing attribute pool`); + continue; + } + // create an array with key kevisions + // key revisions always save the full pad atext + const head = pad.getHeadRevisionNumber(); + const keyRevisions = []; + for (let rev = 0; rev < head; rev += 100) { + keyRevisions.push(rev); + } + + // run through all key revisions + for (const keyRev of keyRevisions) { + // create an array of revisions we need till the next keyRevision or the End + const revisionsNeeded = []; + for (let rev = keyRev; rev <= keyRev + 100 && rev <= head; rev++) { + revisionsNeeded.push(rev); + } + + // this array will hold all revision changesets + const revisions = []; + + // run through all needed revisions and get them from the database + for (const revNum of revisionsNeeded) { + const revision = await db.get(`pad:${pad.id}:revs:${revNum}`); + revisions[revNum] = revision; + } + + // check if the revision exists + if (revisions[keyRev] == null) { + console.error(`[${pad.id}] Missing revision ${keyRev}`); continue; } - // create an array with key kevisions - // key revisions always save the full pad atext - const head = pad.getHeadRevisionNumber(); - const keyRevisions = []; - for (let rev = 0; rev < head; rev += 100) { - keyRevisions.push(rev); + + // check if there is a atext in the keyRevisions + let {meta: {atext} = {}} = revisions[keyRev]; + if (atext == null) { + console.error(`[${pad.id}] Missing atext in revision ${keyRev}`); + continue; } - // run through all key revisions - for (const keyRev of keyRevisions) { - // create an array of revisions we need till the next keyRevision or the End - const revisionsNeeded = []; - for (let rev = keyRev; rev <= keyRev + 100 && rev <= head; rev++) { - revisionsNeeded.push(rev); - } - - // this array will hold all revision changesets - const revisions = []; - - // run through all needed revisions and get them from the database - for (const revNum of revisionsNeeded) { - const revision = await db.get(`pad:${pad.id}:revs:${revNum}`); - revisions[revNum] = revision; - } - - // check if the revision exists - if (revisions[keyRev] == null) { - console.error(`[${pad.id}] Missing revision ${keyRev}`); - continue; - } - - // check if there is a atext in the keyRevisions - let {meta: {atext} = {}} = revisions[keyRev]; - if (atext == null) { - console.error(`[${pad.id}] Missing atext in revision ${keyRev}`); - continue; - } - - const apool = pad.pool; - for (let rev = keyRev + 1; rev <= keyRev + 100 && rev <= head; rev++) { - try { - const cs = revisions[rev].changeset; - atext = Changeset.applyToAText(cs, atext, apool); - revTestedCount++; - } catch (e) { - console.error(`[${pad.id}] Bad changeset at revision ${rev} - ${e.message}`); - } + const apool = pad.pool; + for (let rev = keyRev + 1; rev <= keyRev + 100 && rev <= head; rev++) { + try { + const cs = revisions[rev].changeset; + atext = Changeset.applyToAText(cs, atext, apool); + revTestedCount++; + } catch (e) { + console.error(`[${pad.id}] Bad changeset at revision ${rev} - ${e.message}`); } } } - if (revTestedCount === 0) { - throw new Error('No revisions tested'); - } - console.log(`Finished: Tested ${revTestedCount} revisions`); - } catch (err) { - console.trace(err); - throw err; } + if (revTestedCount === 0) { + throw new Error('No revisions tested'); + } + console.log(`Finished: Tested ${revTestedCount} revisions`); })(); diff --git a/bin/checkPad.js b/bin/checkPad.js index 56464648b..20b3fa226 100644 --- a/bin/checkPad.js +++ b/bin/checkPad.js @@ -19,74 +19,69 @@ let checkRevisionCount = 0; (async () => { await util.promisify(npm.load)({}); - try { - // initialize database - require('ep_etherpad-lite/node/utils/Settings'); - const db = require('ep_etherpad-lite/node/db/DB'); - await db.init(); + // initialize database + require('ep_etherpad-lite/node/utils/Settings'); + const db = require('ep_etherpad-lite/node/db/DB'); + await db.init(); - // load modules - const Changeset = require('ep_etherpad-lite/static/js/Changeset'); - const padManager = require('ep_etherpad-lite/node/db/PadManager'); + // load modules + const Changeset = require('ep_etherpad-lite/static/js/Changeset'); + const padManager = require('ep_etherpad-lite/node/db/PadManager'); - const exists = await padManager.doesPadExists(padId); - if (!exists) throw new Error('Pad does not exist'); + const exists = await padManager.doesPadExists(padId); + if (!exists) throw new Error('Pad does not exist'); - // get the pad - const pad = await padManager.getPad(padId); + // get the pad + const pad = await padManager.getPad(padId); - // create an array with key revisions - // key revisions always save the full pad atext - const head = pad.getHeadRevisionNumber(); - const keyRevisions = []; - for (let rev = 0; rev < head; rev += 100) { - keyRevisions.push(rev); + // create an array with key revisions + // key revisions always save the full pad atext + const head = pad.getHeadRevisionNumber(); + const keyRevisions = []; + for (let rev = 0; rev < head; rev += 100) { + keyRevisions.push(rev); + } + + // run through all key revisions + for (let keyRev of keyRevisions) { + keyRev = parseInt(keyRev); + // create an array of revisions we need till the next keyRevision or the End + const revisionsNeeded = []; + for (let rev = keyRev; rev <= keyRev + 100 && rev <= head; rev++) { + revisionsNeeded.push(rev); } - // run through all key revisions - for (let keyRev of keyRevisions) { - keyRev = parseInt(keyRev); - // create an array of revisions we need till the next keyRevision or the End - const revisionsNeeded = []; - for (let rev = keyRev; rev <= keyRev + 100 && rev <= head; rev++) { - revisionsNeeded.push(rev); - } + // this array will hold all revision changesets + const revisions = []; - // this array will hold all revision changesets - const revisions = []; + // run through all needed revisions and get them from the database + for (const revNum of revisionsNeeded) { + const revision = await db.get(`pad:${padId}:revs:${revNum}`); + revisions[revNum] = revision; + } - // run through all needed revisions and get them from the database - for (const revNum of revisionsNeeded) { - const revision = await db.get(`pad:${padId}:revs:${revNum}`); - revisions[revNum] = revision; - } + // check if the pad has a pool + if (pad.pool == null) throw new Error('Attribute pool is missing'); - // check if the pad has a pool - if (pad.pool == null) throw new Error('Attribute pool is missing'); + // check if there is an atext in the keyRevisions + let {meta: {atext} = {}} = revisions[keyRev] || {}; + if (atext == null) { + console.error(`No atext in key revision ${keyRev}`); + continue; + } - // check if there is an atext in the keyRevisions - let {meta: {atext} = {}} = revisions[keyRev] || {}; - if (atext == null) { - console.error(`No atext in key revision ${keyRev}`); + const apool = pad.pool; + + for (let rev = keyRev + 1; rev <= keyRev + 100 && rev <= head; rev++) { + checkRevisionCount++; + try { + const cs = revisions[rev].changeset; + atext = Changeset.applyToAText(cs, atext, apool); + } catch (e) { + console.error(`Bad changeset at revision ${rev} - ${e.message}`); continue; } - - const apool = pad.pool; - - for (let rev = keyRev + 1; rev <= keyRev + 100 && rev <= head; rev++) { - checkRevisionCount++; - try { - const cs = revisions[rev].changeset; - atext = Changeset.applyToAText(cs, atext, apool); - } catch (e) { - console.error(`Bad changeset at revision ${rev} - ${e.message}`); - continue; - } - } - console.log(`Finished: Checked ${checkRevisionCount} revisions`); } - } catch (err) { - console.trace(err); - throw err; + console.log(`Finished: Checked ${checkRevisionCount} revisions`); } })(); diff --git a/bin/extractPadData.js b/bin/extractPadData.js index 58fe50a42..181a8c6bf 100644 --- a/bin/extractPadData.js +++ b/bin/extractPadData.js @@ -21,54 +21,49 @@ const util = require('util'); (async () => { await util.promisify(npm.load)({}); - try { - // initialize database - require('ep_etherpad-lite/node/utils/Settings'); - const db = require('ep_etherpad-lite/node/db/DB'); - await db.init(); + // initialize database + require('ep_etherpad-lite/node/utils/Settings'); + const db = require('ep_etherpad-lite/node/db/DB'); + await db.init(); - // load extra modules - const dirtyDB = require('ep_etherpad-lite/node_modules/dirty'); - const padManager = require('ep_etherpad-lite/node/db/PadManager'); + // load extra modules + const dirtyDB = require('ep_etherpad-lite/node_modules/dirty'); + const padManager = require('ep_etherpad-lite/node/db/PadManager'); - // initialize output database - const dirty = dirtyDB(`${padId}.db`); + // initialize output database + const dirty = dirtyDB(`${padId}.db`); - // Promise wrapped get and set function - const wrapped = db.db.db.wrappedDB; - const get = util.promisify(wrapped.get.bind(wrapped)); - const set = util.promisify(dirty.set.bind(dirty)); + // Promise wrapped get and set function + const wrapped = db.db.db.wrappedDB; + const get = util.promisify(wrapped.get.bind(wrapped)); + const set = util.promisify(dirty.set.bind(dirty)); - // array in which required key values will be accumulated - const neededDBValues = [`pad:${padId}`]; + // array in which required key values will be accumulated + const neededDBValues = [`pad:${padId}`]; - // get the actual pad object - const pad = await padManager.getPad(padId); + // get the actual pad object + const pad = await padManager.getPad(padId); - // add all authors - neededDBValues.push(...pad.getAllAuthors().map((author) => `globalAuthor:${author}`)); + // add all authors + neededDBValues.push(...pad.getAllAuthors().map((author) => `globalAuthor:${author}`)); - // add all revisions - for (let rev = 0; rev <= pad.head; ++rev) { - neededDBValues.push(`pad:${padId}:revs:${rev}`); - } - - // add all chat values - for (let chat = 0; chat <= pad.chatHead; ++chat) { - neededDBValues.push(`pad:${padId}:chat:${chat}`); - } - - for (const dbkey of neededDBValues) { - let dbvalue = await get(dbkey); - if (dbvalue && typeof dbvalue !== 'object') { - dbvalue = JSON.parse(dbvalue); - } - await set(dbkey, dbvalue); - } - - console.log('finished'); - } catch (err) { - console.error(err); - throw err; + // add all revisions + for (let rev = 0; rev <= pad.head; ++rev) { + neededDBValues.push(`pad:${padId}:revs:${rev}`); } + + // add all chat values + for (let chat = 0; chat <= pad.chatHead; ++chat) { + neededDBValues.push(`pad:${padId}:chat:${chat}`); + } + + for (const dbkey of neededDBValues) { + let dbvalue = await get(dbkey); + if (dbvalue && typeof dbvalue !== 'object') { + dbvalue = JSON.parse(dbvalue); + } + await set(dbkey, dbvalue); + } + + console.log('finished'); })(); From c622894fe0b6b7d17f5e286b9c2b2d85a3408b04 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 9 Jan 2021 03:08:50 -0500 Subject: [PATCH 05/18] bin scripts: Promisify db.init and db.close --- bin/importSqlFile.js | 59 +++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/bin/importSqlFile.js b/bin/importSqlFile.js index b02e9e10c..5927d82ce 100644 --- a/bin/importSqlFile.js +++ b/bin/importSqlFile.js @@ -71,42 +71,35 @@ const unescape = (val) => { if (!sqlFile) throw new Error('Use: node importSqlFile.js $SQLFILE'); log('initializing db'); - db.init((err) => { - // there was an error while initializing the database, output it and stop - if (err) { - throw err; - } else { - log('done'); + await util.promisify(db.init.bind(db))(); + log('done'); - log('open output file...'); - const lines = fs.readFileSync(sqlFile, 'utf8').split('\n'); + log('open output file...'); + const lines = fs.readFileSync(sqlFile, 'utf8').split('\n'); - const count = lines.length; - let keyNo = 0; + const count = lines.length; + let keyNo = 0; - process.stdout.write(`Start importing ${count} keys...\n`); - lines.forEach((l) => { - if (l.substr(0, 27) === 'REPLACE INTO store VALUES (') { - const pos = l.indexOf("', '"); - const key = l.substr(28, pos - 28); - let value = l.substr(pos + 3); - value = value.substr(0, value.length - 2); - console.log(`key: ${key} val: ${value}`); - console.log(`unval: ${unescape(value)}`); - db.set(key, unescape(value), null); - keyNo++; - if (keyNo % 1000 === 0) { - process.stdout.write(` ${keyNo}/${count}\n`); - } - } - }); - process.stdout.write('\n'); - process.stdout.write('done. waiting for db to finish transaction. ' + - 'depended on dbms this may take some time..\n'); - - db.close(() => { - log(`finished, imported ${keyNo} keys.`); - }); + process.stdout.write(`Start importing ${count} keys...\n`); + lines.forEach((l) => { + if (l.substr(0, 27) === 'REPLACE INTO store VALUES (') { + const pos = l.indexOf("', '"); + const key = l.substr(28, pos - 28); + let value = l.substr(pos + 3); + value = value.substr(0, value.length - 2); + console.log(`key: ${key} val: ${value}`); + console.log(`unval: ${unescape(value)}`); + db.set(key, unescape(value), null); + keyNo++; + if (keyNo % 1000 === 0) { + process.stdout.write(` ${keyNo}/${count}\n`); + } } }); + process.stdout.write('\n'); + process.stdout.write('done. waiting for db to finish transaction. ' + + 'depended on dbms this may take some time..\n'); + + await util.promisify(db.close.bind(db))(); + log(`finished, imported ${keyNo} keys.`); })(); From 69efd16a6db1794ae3a719740d296a035e79c7f0 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 17 Jan 2021 03:58:28 -0500 Subject: [PATCH 06/18] bin/rebuildPad.js: Add missing calls to `util.callbackify` --- bin/rebuildPad.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index 0034fe336..83c9bf342 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -29,7 +29,7 @@ async.series([ (callback) => { // Get a handle into the database db = require('ep_etherpad-lite/node/db/DB'); - db.init(callback); + util.callbackify(db.init)(callback); }, (callback) => { Pad = require('ep_etherpad-lite/node/db/Pad').Pad; @@ -44,10 +44,10 @@ async.series([ if (!PadManager.isValidPadId(newPadId)) { throw new Error('Cannot create a pad with that id as it is invalid'); } - PadManager.doesPadExists(newPadId, (err, exists) => { + util.callbackify(PadManager.doesPadExist)(newPadId, (err, exists) => { if (exists) throw new Error('Cannot create a pad with that id as it already exists'); }); - PadManager.getPad(padId, (err, pad) => { + util.callbackify(PadManager.getPad)(padId, (err, pad) => { oldPad = pad; newPad = new Pad(newPadId); callback(); From 72c2abab8d1d5477075c7c09117c55136cd461aa Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 17 Jan 2021 04:19:02 -0500 Subject: [PATCH 07/18] bin/rebuildPad.js: Fix sequencing of asynchronous functions --- bin/rebuildPad.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index 83c9bf342..0f929ef75 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -45,9 +45,14 @@ async.series([ throw new Error('Cannot create a pad with that id as it is invalid'); } util.callbackify(PadManager.doesPadExist)(newPadId, (err, exists) => { + if (err != null) return callback(err); if (exists) throw new Error('Cannot create a pad with that id as it already exists'); + callback(); }); + }, + (callback) => { util.callbackify(PadManager.getPad)(padId, (err, pad) => { + if (err) return callback(err); oldPad = pad; newPad = new Pad(newPadId); callback(); @@ -102,12 +107,11 @@ async.series([ newPad.savedRevisions = newSavedRevisions; callback(); }, + // Save the source pad + (callback) => db.db.set(`pad:${newPadId}`, newPad, callback), (callback) => { - // Save the source pad - db.db.set(`pad:${newPadId}`, newPad, (err) => { - console.log(`Created: Source Pad: pad:${newPadId}`); - util.callbackify(newPad.saveToDatabase.bind(newPad))(callback); - }); + console.log(`Created: Source Pad: pad:${newPadId}`); + util.callbackify(newPad.saveToDatabase.bind(newPad))(callback); }, ], (err) => { if (err) throw err; From 5b519b9a9cf2d5fe8412bbba56ac649f8c0fc618 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 29 Jan 2021 17:01:04 -0500 Subject: [PATCH 08/18] bin/rebuildPad.js: Asyncify --- bin/rebuildPad.js | 160 ++++++++++++++++++++-------------------------- 1 file changed, 68 insertions(+), 92 deletions(-) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index 0f929ef75..3e6e6e5fa 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -13,7 +13,6 @@ if (process.argv.length !== 4 && process.argv.length !== 5) { throw new Error('Use: node bin/repairPad.js $PADID $REV [$NEWPADID]'); } -const async = require('ep_etherpad-lite/node_modules/async'); const npm = require('ep_etherpad-lite/node_modules/npm'); const util = require('util'); @@ -21,99 +20,76 @@ const padId = process.argv[2]; const newRevHead = process.argv[3]; const newPadId = process.argv[4] || `${padId}-rebuilt`; -let db, oldPad, newPad; -let Pad, PadManager; +(async () => { + await util.promisify(npm.load)({}); -async.series([ - (callback) => npm.load({}, callback), - (callback) => { - // Get a handle into the database - db = require('ep_etherpad-lite/node/db/DB'); - util.callbackify(db.init)(callback); - }, - (callback) => { - Pad = require('ep_etherpad-lite/node/db/Pad').Pad; - PadManager = require('ep_etherpad-lite/node/db/PadManager'); - // Get references to the original pad and to a newly created pad - // HACK: This is a standalone script, so we want to write everything - // out to the database immediately. The only problem with this is - // that a driver (like the mysql driver) can hardcode these values. - db.db.db.settings = {cache: 0, writeInterval: 0, json: true}; - // Validate the newPadId if specified and that a pad with that ID does - // not already exist to avoid overwriting it. - if (!PadManager.isValidPadId(newPadId)) { - throw new Error('Cannot create a pad with that id as it is invalid'); + const db = require('ep_etherpad-lite/node/db/DB'); + await db.init(); + + const Pad = require('ep_etherpad-lite/node/db/Pad').Pad; + const PadManager = require('ep_etherpad-lite/node/db/PadManager'); + // Get references to the original pad and to a newly created pad + // HACK: This is a standalone script, so we want to write everything + // out to the database immediately. The only problem with this is + // that a driver (like the mysql driver) can hardcode these values. + db.db.db.settings = {cache: 0, writeInterval: 0, json: true}; + // Validate the newPadId if specified and that a pad with that ID does + // not already exist to avoid overwriting it. + if (!PadManager.isValidPadId(newPadId)) { + throw new Error('Cannot create a pad with that id as it is invalid'); + } + const exists = await PadManager.doesPadExist(newPadId); + if (exists) throw new Error('Cannot create a pad with that id as it already exists'); + + const oldPad = await PadManager.getPad(padId); + const newPad = new Pad(newPadId); + + // Clone all Chat revisions + const chatHead = oldPad.chatHead; + await Promise.all([...Array(chatHead + 1).keys()].map(async (i) => { + const chat = await db.get(`pad:${padId}:chat:${i}`); + await db.set(`pad:${newPadId}:chat:${i}`, chat); + console.log(`Created: Chat Revision: pad:${newPadId}:chat:${i}`); + })); + + // Rebuild Pad from revisions up to and including the new revision head + const AuthorManager = require('ep_etherpad-lite/node/db/AuthorManager'); + const Changeset = require('ep_etherpad-lite/static/js/Changeset'); + // Author attributes are derived from changesets, but there can also be + // non-author attributes with specific mappings that changesets depend on + // and, AFAICT, cannot be recreated any other way + newPad.pool.numToAttrib = oldPad.pool.numToAttrib; + for (let curRevNum = 0; curRevNum <= newRevHead; curRevNum++) { + const rev = await db.get(`pad:${padId}:revs:${curRevNum}`); + if (rev.meta) { + throw new Error('The specified revision number could not be found.'); } - util.callbackify(PadManager.doesPadExist)(newPadId, (err, exists) => { - if (err != null) return callback(err); - if (exists) throw new Error('Cannot create a pad with that id as it already exists'); - callback(); - }); - }, - (callback) => { - util.callbackify(PadManager.getPad)(padId, (err, pad) => { - if (err) return callback(err); - oldPad = pad; - newPad = new Pad(newPadId); - callback(); - }); - }, - (callback) => { - // Clone all Chat revisions - const chatHead = oldPad.chatHead; - for (let i = 0, curHeadNum = 0; i <= chatHead; i++) { - db.db.get(`pad:${padId}:chat:${i}`, (err, chat) => { - db.db.set(`pad:${newPadId}:chat:${curHeadNum++}`, chat); - console.log(`Created: Chat Revision: pad:${newPadId}:chat:${curHeadNum}`); - }); + const newRevNum = ++newPad.head; + const newRevId = `pad:${newPad.id}:revs:${newRevNum}`; + await Promise.all([ + db.set(newRevId, rev), + AuthorManager.addPad(rev.meta.author, newPad.id), + ]); + newPad.atext = Changeset.applyToAText(rev.changeset, newPad.atext, newPad.pool); + console.log(`Created: Revision: pad:${newPad.id}:revs:${newRevNum}`); + } + + // Add saved revisions up to the new revision head + console.log(newPad.head); + const newSavedRevisions = []; + for (const savedRev of oldPad.savedRevisions) { + if (savedRev.revNum <= newRevHead) { + newSavedRevisions.push(savedRev); + console.log(`Added: Saved Revision: ${savedRev.revNum}`); } - callback(); - }, - (callback) => { - // Rebuild Pad from revisions up to and including the new revision head - const AuthorManager = require('ep_etherpad-lite/node/db/AuthorManager'); - const Changeset = require('ep_etherpad-lite/static/js/Changeset'); - // Author attributes are derived from changesets, but there can also be - // non-author attributes with specific mappings that changesets depend on - // and, AFAICT, cannot be recreated any other way - newPad.pool.numToAttrib = oldPad.pool.numToAttrib; - for (let curRevNum = 0; curRevNum <= newRevHead; curRevNum++) { - db.db.get(`pad:${padId}:revs:${curRevNum}`, (err, rev) => { - if (rev.meta) { - throw new Error('The specified revision number could not be found.'); - } - const newRevNum = ++newPad.head; - const newRevId = `pad:${newPad.id}:revs:${newRevNum}`; - db.db.set(newRevId, rev); - AuthorManager.addPad(rev.meta.author, newPad.id); - newPad.atext = Changeset.applyToAText(rev.changeset, newPad.atext, newPad.pool); - console.log(`Created: Revision: pad:${newPad.id}:revs:${newRevNum}`); - if (newRevNum === newRevHead) { - callback(); - } - }); - } - }, - (callback) => { - // Add saved revisions up to the new revision head - console.log(newPad.head); - const newSavedRevisions = []; - for (const savedRev of oldPad.savedRevisions) { - if (savedRev.revNum <= newRevHead) { - newSavedRevisions.push(savedRev); - console.log(`Added: Saved Revision: ${savedRev.revNum}`); - } - } - newPad.savedRevisions = newSavedRevisions; - callback(); - }, + } + newPad.savedRevisions = newSavedRevisions; + // Save the source pad - (callback) => db.db.set(`pad:${newPadId}`, newPad, callback), - (callback) => { - console.log(`Created: Source Pad: pad:${newPadId}`); - util.callbackify(newPad.saveToDatabase.bind(newPad))(callback); - }, -], (err) => { - if (err) throw err; + await db.set(`pad:${newPadId}`, newPad); + + console.log(`Created: Source Pad: pad:${newPadId}`); + await newPad.saveToDatabase(); + console.info('finished'); -}); +})(); From 809dc6e367f1db7f25e08dcc04fda5eb7e9b1b0f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 29 Jan 2021 17:15:00 -0500 Subject: [PATCH 09/18] bin/rebuildPad.js: PadManager must be loaded before Pad There is a circular dependency between the two; loading PadManager first ensures that PadManager's Pad variable is not undefined. --- bin/rebuildPad.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index 3e6e6e5fa..f5043b85b 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -26,8 +26,8 @@ const newPadId = process.argv[4] || `${padId}-rebuilt`; const db = require('ep_etherpad-lite/node/db/DB'); await db.init(); - const Pad = require('ep_etherpad-lite/node/db/Pad').Pad; const PadManager = require('ep_etherpad-lite/node/db/PadManager'); + const Pad = require('ep_etherpad-lite/node/db/Pad').Pad; // Get references to the original pad and to a newly created pad // HACK: This is a standalone script, so we want to write everything // out to the database immediately. The only problem with this is From bf209ddad38ce12f446093092c7f5899a6d7144b Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 29 Jan 2021 17:19:40 -0500 Subject: [PATCH 10/18] bin/rebuildPad.js: Close the database when done This prevents loss of data due to unflushed writes. --- bin/rebuildPad.js | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index f5043b85b..f541a4ce4 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -91,5 +91,6 @@ const newPadId = process.argv[4] || `${padId}-rebuilt`; console.log(`Created: Source Pad: pad:${newPadId}`); await newPad.saveToDatabase(); + await db.shutdown(); console.info('finished'); })(); From 846e3e9fbdce80c3ff84e7ae94d5d7d698e4978a Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 29 Jan 2021 17:20:25 -0500 Subject: [PATCH 11/18] bin/rebuildPad.js: Don't overwrite DB settings There's no need, and setting `json` to true breaks databases that do their own serialization of ECMAScript objects. --- bin/rebuildPad.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index f541a4ce4..870bbcc4a 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -28,11 +28,6 @@ const newPadId = process.argv[4] || `${padId}-rebuilt`; const PadManager = require('ep_etherpad-lite/node/db/PadManager'); const Pad = require('ep_etherpad-lite/node/db/Pad').Pad; - // Get references to the original pad and to a newly created pad - // HACK: This is a standalone script, so we want to write everything - // out to the database immediately. The only problem with this is - // that a driver (like the mysql driver) can hardcode these values. - db.db.db.settings = {cache: 0, writeInterval: 0, json: true}; // Validate the newPadId if specified and that a pad with that ID does // not already exist to avoid overwriting it. if (!PadManager.isValidPadId(newPadId)) { From 0ad0160b7ce8e17053c6c255cd34ccd845d291fc Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 29 Jan 2021 17:23:11 -0500 Subject: [PATCH 12/18] bin/rebuildPad.js: Fix check for existing rev --- bin/rebuildPad.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bin/rebuildPad.js b/bin/rebuildPad.js index 870bbcc4a..1bc942fe2 100644 --- a/bin/rebuildPad.js +++ b/bin/rebuildPad.js @@ -56,9 +56,7 @@ const newPadId = process.argv[4] || `${padId}-rebuilt`; newPad.pool.numToAttrib = oldPad.pool.numToAttrib; for (let curRevNum = 0; curRevNum <= newRevHead; curRevNum++) { const rev = await db.get(`pad:${padId}:revs:${curRevNum}`); - if (rev.meta) { - throw new Error('The specified revision number could not be found.'); - } + if (!rev || !rev.meta) throw new Error('The specified revision number could not be found.'); const newRevNum = ++newPad.head; const newRevId = `pad:${newPad.id}:revs:${newRevNum}`; await Promise.all([ From 86ceb2b610a2a9a15fedc70b5690017e41fee5fe Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 14 Dec 2020 20:45:12 -0500 Subject: [PATCH 13/18] server: Exit on unhandled Promise rejection --- src/node/server.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/node/server.js b/src/node/server.js index 3e3b25d21..d14fca92d 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -77,6 +77,9 @@ exports.start = async () => { } process.on('uncaughtException', exports.exit); + // As of v14, Node.js does not exit when there is an unhandled Promise rejection. Convert an + // unhandled rejection into an uncaught exception, which does cause Node.js to exit. + process.on('unhandledRejection', (err) => { throw err; }); /* * Connect graceful shutdown with sigint and uncaught exception From d339f2a6715f154bae7d68e52393377f0030f18f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 22 Dec 2020 00:45:33 -0500 Subject: [PATCH 14/18] server: Perform init after adding uncaught exception handler This avoids an unnecessary `try` block. --- src/node/server.js | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/node/server.js b/src/node/server.js index d14fca92d..ece5ed25b 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -60,22 +60,6 @@ exports.start = async () => { stats.gauge('memoryUsage', () => process.memoryUsage().rss); stats.gauge('memoryUsageHeap', () => process.memoryUsage().heapUsed); - await util.promisify(npm.load)(); - - try { - await db.init(); - await plugins.update(); - console.info(`Installed plugins: ${plugins.formatPluginsWithVersion()}`); - console.debug(`Installed parts:\n${plugins.formatParts()}`); - console.debug(`Installed hooks:\n${plugins.formatHooks()}`); - await hooks.aCallAll('loadSettings', {settings}); - await hooks.aCallAll('createServer'); - } catch (e) { - console.error(`exception thrown: ${e.message}`); - if (e.stack) console.log(e.stack); - process.exit(1); - } - process.on('uncaughtException', exports.exit); // As of v14, Node.js does not exit when there is an unhandled Promise rejection. Convert an // unhandled rejection into an uncaught exception, which does cause Node.js to exit. @@ -105,6 +89,15 @@ exports.start = async () => { // Pass undefined to exports.exit because this is not an abnormal termination. process.on('SIGTERM', () => exports.exit()); + await util.promisify(npm.load)(); + await db.init(); + await plugins.update(); + console.info(`Installed plugins: ${plugins.formatPluginsWithVersion()}`); + console.debug(`Installed parts:\n${plugins.formatParts()}`); + console.debug(`Installed hooks:\n${plugins.formatHooks()}`); + await hooks.aCallAll('loadSettings', {settings}); + await hooks.aCallAll('createServer'); + // Return the HTTP server to make it easier to write tests. return express.server; }; From 725023fe5883ebc67723948a98e7c06042216fc0 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 22 Dec 2020 01:01:37 -0500 Subject: [PATCH 15/18] server: Refactor `stop()` to avoid no-async-promise-executor lint error Also log when Etherpad has stopped. --- src/node/server.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/node/server.js b/src/node/server.js index ece5ed25b..38ffce22c 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -106,12 +106,15 @@ exports.stop = async () => { if (stopped) return; stopped = true; console.log('Stopping Etherpad...'); - await new Promise(async (resolve, reject) => { - const id = setTimeout(() => reject(new Error('Timed out waiting for shutdown tasks')), 3000); - await hooks.aCallAll('shutdown'); - clearTimeout(id); - resolve(); - }); + let timeout = null; + await Promise.race([ + hooks.aCallAll('shutdown'), + new Promise((resolve, reject) => { + timeout = setTimeout(() => reject(new Error('Timed out waiting for shutdown tasks')), 3000); + }), + ]); + clearTimeout(timeout); + console.log('Etherpad stopped'); }; exports.exit = async (err) => { From ecdb105bfed80f22ab67ec3640cc09e0019233c6 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 22 Dec 2020 01:08:50 -0500 Subject: [PATCH 16/18] server: Refine process lifetime management Define states and use them to properly handle multiple calls to `start()`, `stop()`, and `exit()`. (Multiple calls to `exit()` can happen if there is an uncaught exception or signal during shutdown.) This should also make it easier to add support for cleanly restarting the server after a shutdown (for tests or via an `/admin` page). --- src/node/server.js | 127 +++++++++++++++++++++++++++++++++------- tests/backend/common.js | 2 +- 2 files changed, 108 insertions(+), 21 deletions(-) diff --git a/src/node/server.js b/src/node/server.js index 38ffce22c..4cfca5fd1 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -44,13 +44,38 @@ const plugins = require('../static/js/pluginfw/plugins'); const settings = require('./utils/Settings'); const util = require('util'); -let started = false; -let stopped = false; +const State = { + INITIAL: 1, + STARTING: 2, + RUNNING: 3, + STOPPING: 4, + STOPPED: 5, + EXITING: 6, + WAITING_FOR_EXIT: 7, +}; +let state = State.INITIAL; + +const runningCallbacks = []; exports.start = async () => { - if (started) return express.server; - started = true; - if (stopped) throw new Error('restart not supported'); + switch (state) { + case State.INITIAL: + break; + case State.STARTING: + await new Promise((resolve) => runningCallbacks.push(resolve)); + // fall through + case State.RUNNING: + return express.server; + case State.STOPPING: + case State.STOPPED: + case State.EXITING: + case State.WAITING_FOR_EXIT: + throw new Error('restart not supported'); + default: + throw new Error(`unknown State: ${state.toString()}`); + } + console.log('Starting Etherpad...'); + state = State.STARTING; // Check if Etherpad version is up-to-date UpdateCheck.check(); @@ -86,8 +111,7 @@ exports.start = async () => { process.on('SIGINT', exports.exit); // When running as PID1 (e.g. in docker container) allow graceful shutdown on SIGTERM c.f. #3265. - // Pass undefined to exports.exit because this is not an abnormal termination. - process.on('SIGTERM', () => exports.exit()); + process.on('SIGTERM', exports.exit); await util.promisify(npm.load)(); await db.init(); @@ -98,14 +122,36 @@ exports.start = async () => { await hooks.aCallAll('loadSettings', {settings}); await hooks.aCallAll('createServer'); + console.log('Etherpad is running'); + state = State.RUNNING; + while (runningCallbacks.length > 0) setImmediate(runningCallbacks.pop()); + // Return the HTTP server to make it easier to write tests. return express.server; }; +const stoppedCallbacks = []; exports.stop = async () => { - if (stopped) return; - stopped = true; + switch (state) { + case State.STARTING: + await exports.start(); + // Don't fall through to State.RUNNING in case another caller is also waiting for startup. + return await exports.stop(); + case State.RUNNING: + break; + case State.STOPPING: + await new Promise((resolve) => stoppedCallbacks.push(resolve)); + // fall through + case State.INITIAL: + case State.STOPPED: + case State.EXITING: + case State.WAITING_FOR_EXIT: + return; + default: + throw new Error(`unknown State: ${state.toString()}`); + } console.log('Stopping Etherpad...'); + state = State.STOPPING; let timeout = null; await Promise.race([ hooks.aCallAll('shutdown'), @@ -115,21 +161,62 @@ exports.stop = async () => { ]); clearTimeout(timeout); console.log('Etherpad stopped'); + state = State.STOPPED; + while (stoppedCallbacks.length > 0) setImmediate(stoppedCallbacks.pop()); }; -exports.exit = async (err) => { - let exitCode = 0; - if (err) { - exitCode = 1; - console.error(err.stack ? err.stack : err); +const exitCallbacks = []; +let exitCalled = false; +exports.exit = async (err = null) => { + /* eslint-disable no-process-exit */ + if (err === 'SIGTERM') { + // Termination from SIGTERM is not treated as an abnormal termination. + console.log('Received SIGTERM signal'); + err = null; + } else if (err != null) { + console.error(err.stack || err.toString()); + process.exitCode = 1; + if (exitCalled) { + console.error('Error occurred while waiting to exit. Forcing an immediate unclean exit...'); + process.exit(1); + } } - try { - await exports.stop(); - } catch (err) { - exitCode = 1; - console.error(err.stack ? err.stack : err); + exitCalled = true; + switch (state) { + case State.STARTING: + case State.RUNNING: + case State.STOPPING: + await exports.stop(); + // Don't fall through to State.STOPPED in case another caller is also waiting for stop(). + // Don't pass err to exports.exit() because this err has already been processed. (If err is + // passed again to exit() then exit() will think that a second error occurred while exiting.) + return await exports.exit(); + case State.INITIAL: + case State.STOPPED: + break; + case State.EXITING: + await new Promise((resolve) => exitCallbacks.push(resolve)); + // fall through + case State.WAITING_FOR_EXIT: + return; + default: + throw new Error(`unknown State: ${state.toString()}`); } - process.exit(exitCode); + console.log('Exiting...'); + state = State.EXITING; + while (exitCallbacks.length > 0) setImmediate(exitCallbacks.pop()); + // Node.js should exit on its own without further action. Add a timeout to force Node.js to exit + // just in case something failed to get cleaned up during the shutdown hook. unref() is called on + // the timeout so that the timeout itself does not prevent Node.js from exiting. + setTimeout(() => { + console.error('Something that should have been cleaned up during the shutdown hook (such as ' + + 'a timer, worker thread, or open connection) is preventing Node.js from exiting'); + console.error('Forcing an unclean exit...'); + process.exit(1); + }, 5000).unref(); + console.log('Waiting for Node.js to exit...'); + state = State.WAITING_FOR_EXIT; + /* eslint-enable no-process-exit */ }; if (require.main === module) exports.start(); diff --git a/tests/backend/common.js b/tests/backend/common.js index 22a84c097..642f83afe 100644 --- a/tests/backend/common.js +++ b/tests/backend/common.js @@ -50,10 +50,10 @@ exports.init = async function () { after(async function () { webaccess.authnFailureDelayMs = backups.authnFailureDelayMs; - await server.stop(); // Note: This does not unset settings that were added. Object.assign(settings, backups.settings); log4js.setGlobalLogLevel(logLevel); + await server.exit(); }); return exports.agent; From ba81ead1018e2c78ae724226bb7a0b845544b62f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 22 Dec 2020 01:21:43 -0500 Subject: [PATCH 17/18] server: Remove all other signal listeners --- src/node/server.js | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/node/server.js b/src/node/server.js index 4cfca5fd1..1a6f9cbe5 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -56,6 +56,13 @@ const State = { let state = State.INITIAL; +const removeSignalListener = (signal, listener) => { + console.debug(`Removing ${signal} listener because it might interfere with shutdown tasks. ` + + `Function code:\n${listener.toString()}\n` + + `Current stack:\n${(new Error()).stack.split('\n').slice(1).join('\n')}`); + process.off(signal, listener); +}; + const runningCallbacks = []; exports.start = async () => { switch (state) { @@ -90,28 +97,21 @@ exports.start = async () => { // unhandled rejection into an uncaught exception, which does cause Node.js to exit. process.on('unhandledRejection', (err) => { throw err; }); - /* - * Connect graceful shutdown with sigint and uncaught exception - * - * Until Etherpad 1.7.5, process.on('SIGTERM') and process.on('SIGINT') were - * not hooked up under Windows, because old nodejs versions did not support - * them. - * - * According to nodejs 6.x documentation, it is now safe to do so. This - * allows to gracefully close the DB connection when hitting CTRL+C under - * Windows, for example. - * - * Source: https://nodejs.org/docs/latest-v6.x/api/process.html#process_signal_events - * - * - SIGTERM is not supported on Windows, it can be listened on. - * - SIGINT from the terminal is supported on all platforms, and can usually - * be generated with +C (though this may be configurable). It is not - * generated when terminal raw mode is enabled. - */ - process.on('SIGINT', exports.exit); - - // When running as PID1 (e.g. in docker container) allow graceful shutdown on SIGTERM c.f. #3265. - process.on('SIGTERM', exports.exit); + for (const signal of ['SIGINT', 'SIGTERM']) { + // Forcibly remove other signal listeners to prevent them from terminating node before we are + // done cleaning up. See https://github.com/andywer/threads.js/pull/329 for an example of a + // problematic listener. This means that exports.exit is solely responsible for performing all + // necessary cleanup tasks. + for (const listener of process.listeners(signal)) { + removeSignalListener(signal, listener); + } + process.on(signal, exports.exit); + // Prevent signal listeners from being added in the future. + process.on('newListener', (event, listener) => { + if (event !== signal) return; + removeSignalListener(signal, listener); + }); + } await util.promisify(npm.load)(); await db.init(); From 877f0c5883a33c9e513e81fc56d2fa6c3ed66c9e Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 15 Dec 2020 03:23:12 -0500 Subject: [PATCH 18/18] server: Use wtfnode to log reasons why node isn't exiting --- package-lock.json | 8 +++++++- src/node/server.js | 5 +++++ src/package-lock.json | 5 +++++ src/package.json | 3 ++- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4dcc411d9..67ca85767 100644 --- a/package-lock.json +++ b/package-lock.json @@ -870,7 +870,8 @@ "tinycon": "0.0.1", "ueberdb2": "^1.2.5", "underscore": "1.8.3", - "unorm": "1.4.1" + "unorm": "1.4.1", + "wtfnode": "^0.8.4" }, "dependencies": { "@apidevtools/json-schema-ref-parser": { @@ -10577,6 +10578,11 @@ "resolved": "https://registry.npmjs.org/xtend/-/xtend-4.0.2.tgz", "integrity": "sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ==" }, + "wtfnode": { + "version": "0.8.4", + "resolved": "https://registry.npmjs.org/wtfnode/-/wtfnode-0.8.4.tgz", + "integrity": "sha512-64GEKtMt/MUBuAm+8kHqP74ojjafzu00aT0JKsmkIwYmjRQ/odO0yhbzKLm+Z9v1gMla+8dwITRKzTAlHsB+Og==" + }, "yallist": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", diff --git a/src/node/server.js b/src/node/server.js index 1a6f9cbe5..bbe4c7aeb 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -27,6 +27,10 @@ const log4js = require('log4js'); log4js.replaceConsole(); +// wtfnode should be loaded after log4js.replaceConsole() so that it uses log4js for logging, and it +// should be above everything else so that it can hook in before resources are used. +const wtfnode = require('wtfnode'); + /* * early check for version compatibility before calling * any modules that require newer versions of NodeJS @@ -211,6 +215,7 @@ exports.exit = async (err = null) => { setTimeout(() => { console.error('Something that should have been cleaned up during the shutdown hook (such as ' + 'a timer, worker thread, or open connection) is preventing Node.js from exiting'); + wtfnode.dump(); console.error('Forcing an unclean exit...'); process.exit(1); }, 5000).unref(); diff --git a/src/package-lock.json b/src/package-lock.json index 3e04ec287..07441bde4 100644 --- a/src/package-lock.json +++ b/src/package-lock.json @@ -8659,6 +8659,11 @@ "resolved": "https://registry.npmjs.org/ws/-/ws-7.4.2.tgz", "integrity": "sha512-T4tewALS3+qsrpGI/8dqNMLIVdq/g/85U98HPMa6F0m6xTbvhXU6RCQLqPH3+SlomNV/LdY6RXEbBpMH6EOJnA==" }, + "wtfnode": { + "version": "0.8.4", + "resolved": "https://registry.npmjs.org/wtfnode/-/wtfnode-0.8.4.tgz", + "integrity": "sha512-64GEKtMt/MUBuAm+8kHqP74ojjafzu00aT0JKsmkIwYmjRQ/odO0yhbzKLm+Z9v1gMla+8dwITRKzTAlHsB+Og==" + }, "xml2js": { "version": "0.4.23", "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", diff --git a/src/package.json b/src/package.json index bc3e4d22f..816f5b587 100644 --- a/src/package.json +++ b/src/package.json @@ -72,7 +72,8 @@ "tinycon": "0.0.1", "ueberdb2": "^1.2.5", "underscore": "1.8.3", - "unorm": "1.4.1" + "unorm": "1.4.1", + "wtfnode": "^0.8.4" }, "bin": { "etherpad-lite": "node/server.js"