From 46ab00bfe4a9a90b4067f50791a278d4ba7c5489 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Sun, 10 Aug 2025 01:14:25 -0700 Subject: [PATCH] Chore: Sync fuzzer: Re-use the same CLI process for commands run on the same client (#12913) --- .eslintignore | 3 + .gitignore | 3 + packages/app-cli/app/app.ts | 27 +- packages/app-cli/app/command-batch.js | 19 - packages/app-cli/app/command-batch.ts | 79 +++++ packages/app-cli/app/command-server.js | 18 +- packages/app-cli/app/utils/iterateStdin.ts | 54 +++ packages/lib/BaseApplication.ts | 19 +- packages/tools/fuzzer/Client.ts | 330 ++++++++++++++---- packages/tools/fuzzer/ClientPool.ts | 28 +- packages/tools/fuzzer/sync-fuzzer.ts | 19 +- .../tools/fuzzer/utils/openDebugSession.ts | 50 +++ packages/tools/fuzzer/utils/retryWithCount.ts | 15 +- 13 files changed, 518 insertions(+), 146 deletions(-) delete mode 100644 packages/app-cli/app/command-batch.js create mode 100644 packages/app-cli/app/command-batch.ts create mode 100644 packages/app-cli/app/utils/iterateStdin.ts create mode 100644 packages/tools/fuzzer/utils/openDebugSession.ts diff --git a/.eslintignore b/.eslintignore index 034843e1ae..1c4fc04901 100644 --- a/.eslintignore +++ b/.eslintignore @@ -98,6 +98,7 @@ packages/app-cli/app/app.js packages/app-cli/app/base-command.js packages/app-cli/app/command-apidoc.js packages/app-cli/app/command-attach.js +packages/app-cli/app/command-batch.js packages/app-cli/app/command-cat.js packages/app-cli/app/command-config.js packages/app-cli/app/command-cp.js @@ -135,6 +136,7 @@ packages/app-cli/app/gui/StatusBarWidget.js packages/app-cli/app/services/plugins/PluginRunner.js packages/app-cli/app/setupCommand.js packages/app-cli/app/utils/initializeCommandService.js +packages/app-cli/app/utils/iterateStdin.js packages/app-cli/app/utils/shimInitCli.js packages/app-cli/app/utils/testUtils.js packages/app-cli/tests/HtmlToMd.js @@ -1747,6 +1749,7 @@ packages/tools/fuzzer/utils/SeededRandom.js packages/tools/fuzzer/utils/getNumberProperty.js packages/tools/fuzzer/utils/getProperty.js packages/tools/fuzzer/utils/getStringProperty.js +packages/tools/fuzzer/utils/openDebugSession.js packages/tools/fuzzer/utils/retryWithCount.js packages/tools/generate-database-types.js packages/tools/generate-images.js diff --git a/.gitignore b/.gitignore index b070294533..cc91d263e8 100644 --- a/.gitignore +++ b/.gitignore @@ -71,6 +71,7 @@ packages/app-cli/app/app.js packages/app-cli/app/base-command.js packages/app-cli/app/command-apidoc.js packages/app-cli/app/command-attach.js +packages/app-cli/app/command-batch.js packages/app-cli/app/command-cat.js packages/app-cli/app/command-config.js packages/app-cli/app/command-cp.js @@ -108,6 +109,7 @@ packages/app-cli/app/gui/StatusBarWidget.js packages/app-cli/app/services/plugins/PluginRunner.js packages/app-cli/app/setupCommand.js packages/app-cli/app/utils/initializeCommandService.js +packages/app-cli/app/utils/iterateStdin.js packages/app-cli/app/utils/shimInitCli.js packages/app-cli/app/utils/testUtils.js packages/app-cli/tests/HtmlToMd.js @@ -1720,6 +1722,7 @@ packages/tools/fuzzer/utils/SeededRandom.js packages/tools/fuzzer/utils/getNumberProperty.js packages/tools/fuzzer/utils/getProperty.js packages/tools/fuzzer/utils/getStringProperty.js +packages/tools/fuzzer/utils/openDebugSession.js packages/tools/fuzzer/utils/retryWithCount.js packages/tools/generate-database-types.js packages/tools/generate-images.js diff --git a/packages/app-cli/app/app.ts b/packages/app-cli/app/app.ts index d145695e5d..5ac33d57d9 100644 --- a/packages/app-cli/app/app.ts +++ b/packages/app-cli/app/app.ts @@ -9,7 +9,6 @@ import Tag from '@joplin/lib/models/Tag'; import Setting, { Env } from '@joplin/lib/models/Setting'; import { reg } from '@joplin/lib/registry.js'; import { dirname, fileExtension } from '@joplin/lib/path-utils'; -import { splitCommandString } from '@joplin/utils'; import { _ } from '@joplin/lib/locale'; import { pathExists, readFile, readdirSync } from 'fs-extra'; import RevisionService from '@joplin/lib/services/RevisionService'; @@ -19,7 +18,6 @@ import { FolderEntity, NoteEntity } from '@joplin/lib/services/database/types'; import initializeCommandService from './utils/initializeCommandService'; const { cliUtils } = require('./cli-utils.js'); const Cache = require('@joplin/lib/Cache'); -const { splitCommandBatch } = require('@joplin/lib/string-utils'); class Application extends BaseApplication { @@ -381,22 +379,6 @@ class Application extends BaseApplication { return output; } - public async commandList(argv: string[]) { - if (argv.length && argv[0] === 'batch') { - const commands = []; - const commandLines = splitCommandBatch(await readFile(argv[1], 'utf-8')); - - for (const commandLine of commandLines) { - if (!commandLine.trim()) continue; - const splitted = splitCommandString(commandLine.trim()); - commands.push(splitted); - } - return commands; - } else { - return [argv]; - } - } - // We need this special case here because by the time the `version` command // runs, the keychain has already been setup. public checkIfKeychainEnabled(argv: string[]) { @@ -433,15 +415,10 @@ class Application extends BaseApplication { if (argv.length) { this.gui_ = this.dummyGui(); - this.currentFolder_ = await Folder.load(Setting.value('activeFolderId')); - await this.applySettingsSideEffects(); - + await this.refreshCurrentFolder(); try { - const commands = await this.commandList(argv); - for (const command of commands) { - await this.execCommand(command); - } + await this.execCommand(argv); } catch (error) { if (this.showStackTraces_) { console.error(error); diff --git a/packages/app-cli/app/command-batch.js b/packages/app-cli/app/command-batch.js deleted file mode 100644 index dc445382e7..0000000000 --- a/packages/app-cli/app/command-batch.js +++ /dev/null @@ -1,19 +0,0 @@ -const BaseCommand = require('./base-command').default; -const { _ } = require('@joplin/lib/locale'); - -class Command extends BaseCommand { - usage() { - return 'batch '; - } - - description() { - return _('Runs the commands contained in the text file. There should be one command per line.'); - } - - async action() { - // Implementation is in app.js::commandList() - throw new Error('No implemented'); - } -} - -module.exports = Command; diff --git a/packages/app-cli/app/command-batch.ts b/packages/app-cli/app/command-batch.ts new file mode 100644 index 0000000000..b2f18af9f3 --- /dev/null +++ b/packages/app-cli/app/command-batch.ts @@ -0,0 +1,79 @@ +import { splitCommandBatch } from '@joplin/lib/string-utils'; +import BaseCommand from './base-command'; +import { _ } from '@joplin/lib/locale'; +import { splitCommandString } from '@joplin/utils'; +import iterateStdin from './utils/iterateStdin'; +import { readFile } from 'fs-extra'; +import app from './app'; + +interface Options { + 'file-path': string; + options: { + 'continue-on-failure': boolean; + }; +} + +class Command extends BaseCommand { + public usage() { + return 'batch '; + } + + public options() { + return [ + // These are present mostly for testing purposes + ['--continue-on-failure', 'Continue running commands when one command in the batch fails.'], + ]; + } + + public description() { + return _('Runs the commands contained in the text file. There should be one command per line.'); + } + + private streamCommands_ = async function*(filePath: string) { + const processLines = function*(lines: string) { + const commandLines = splitCommandBatch(lines); + + for (const command of commandLines) { + if (!command.trim()) continue; + yield splitCommandString(command.trim()); + } + }; + + if (filePath === '-') { // stdin + // Iterating over standard input conflicts with the CLI app's GUI. + if (app().hasGui()) { + throw new Error(_('Reading commands from standard input is only available in CLI mode.')); + } + + for await (const lines of iterateStdin('command> ')) { + yield* processLines(lines); + } + } else { + const data = await readFile(filePath, 'utf-8'); + yield* processLines(data); + } + }; + + public async action(options: Options) { + let lastError; + for await (const command of this.streamCommands_(options['file-path'])) { + try { + await app().refreshCurrentFolder(); + await app().execCommand(command); + } catch (error) { + if (options.options['continue-on-failure']) { + app().stdout(error.message); + lastError = error; + } else { + throw error; + } + } + } + + if (lastError) { + throw lastError; + } + } +} + +module.exports = Command; diff --git a/packages/app-cli/app/command-server.js b/packages/app-cli/app/command-server.js index 4b0305e60a..994724980f 100644 --- a/packages/app-cli/app/command-server.js +++ b/packages/app-cli/app/command-server.js @@ -14,17 +14,25 @@ class Command extends BaseCommand { return `${_('Start, stop or check the API server. To specify on which port it should run, set the api.port config variable. Commands are (%s).', ['start', 'stop', 'status'].join('|'))} This is an experimental feature - use at your own risks! It is recommended that the server runs off its own separate profile so that no two CLI instances access that profile at the same time. Use --profile to specify the profile path.`; } + options() { + return [ + ['--exit-early', 'Allow the command to exit while the server is still running. The server will still stop when the app exits. Valid only for the `start` subcommand.'], + ['--quiet', 'Log less information to the console. More verbose logs will still be available through log-clipper.txt.'], + ]; + } + async action(args) { const command = args.command; const ClipperServer = require('@joplin/lib/ClipperServer').default; ClipperServer.instance().initialize(); const stdoutFn = (...s) => this.stdout(s.join(' ')); + const ignoreOutputFn = ()=>{}; const clipperLogger = new Logger(); clipperLogger.addTarget('file', { path: `${Setting.value('profileDir')}/log-clipper.txt` }); clipperLogger.addTarget('console', { console: { - info: stdoutFn, - warn: stdoutFn, + info: args.options.quiet ? ignoreOutputFn : stdoutFn, + warn: args.options.quiet ? ignoreOutputFn : stdoutFn, error: stdoutFn, } }); ClipperServer.instance().setDispatch(() => {}); @@ -38,7 +46,11 @@ class Command extends BaseCommand { this.stdout(_('Server is already running on port %d', runningOnPort)); } else { await shim.fsDriver().writeFile(pidPath, process.pid.toString(), 'utf-8'); - await ClipperServer.instance().start(); // Never exit + const promise = ClipperServer.instance().start(); + + if (!args.options['exit-early']) { + await promise; // Never exit + } } } else if (command === 'status') { this.stdout(runningOnPort ? _('Server is running on port %d', runningOnPort) : _('Server is not running.')); diff --git a/packages/app-cli/app/utils/iterateStdin.ts b/packages/app-cli/app/utils/iterateStdin.ts new file mode 100644 index 0000000000..e5bf12de6a --- /dev/null +++ b/packages/app-cli/app/utils/iterateStdin.ts @@ -0,0 +1,54 @@ +import { createInterface } from 'readline/promises'; + +const iterateStdin = async function*(prompt: string) { + let nextLineListeners: (()=> void)[] = []; + const dispatchAllListeners = () => { + const listeners = nextLineListeners; + nextLineListeners = []; + for (const listener of listeners) { + listener(); + } + }; + + const rl = createInterface({ + input: process.stdin, + output: process.stdout, + }); + rl.setPrompt(prompt); + + let buffer: string[] = []; + rl.on('line', (line) => { + buffer.push(line); + dispatchAllListeners(); + }); + + let done = false; + rl.on('close', () => { + done = true; + dispatchAllListeners(); + }); + + const readNextLines = () => { + return new Promise(resolve => { + if (done) { + resolve(null); + } else if (buffer.length > 0) { + resolve(buffer.join('\n')); + buffer = []; + } else { + nextLineListeners.push(() => { + resolve(buffer.join('\n')); + buffer = []; + }); + } + }); + }; + + while (!done) { + rl.prompt(); + const lines = await readNextLines(); + yield lines; + } +}; + +export default iterateStdin; diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts index 7102e133d2..e12d4574e4 100644 --- a/packages/lib/BaseApplication.ts +++ b/packages/lib/BaseApplication.ts @@ -395,17 +395,18 @@ export default class BaseApplication { // - All the calls below are cheap or do nothing if there's nothing // to do. 'syncInfoCache': async () => { + appLogger.info('"syncInfoCache" was changed - setting up encryption related code'); + + await loadMasterKeysFromSettings(EncryptionService.instance()); + const loadedMasterKeyIds = EncryptionService.instance().loadedMasterKeyIds(); + + this.dispatch({ + type: 'MASTERKEY_REMOVE_NOT_LOADED', + ids: loadedMasterKeyIds, + }); + if (this.hasGui()) { - appLogger.info('"syncInfoCache" was changed - setting up encryption related code'); - - await loadMasterKeysFromSettings(EncryptionService.instance()); void DecryptionWorker.instance().scheduleStart(); - const loadedMasterKeyIds = EncryptionService.instance().loadedMasterKeyIds(); - - this.dispatch({ - type: 'MASTERKEY_REMOVE_NOT_LOADED', - ids: loadedMasterKeyIds, - }); // Schedule a sync operation so that items that need to be encrypted // are sent to sync target. diff --git a/packages/tools/fuzzer/Client.ts b/packages/tools/fuzzer/Client.ts index 1785e8167c..a3f154de35 100644 --- a/packages/tools/fuzzer/Client.ts +++ b/packages/tools/fuzzer/Client.ts @@ -1,21 +1,39 @@ import uuid, { createSecureRandom } from '@joplin/lib/uuid'; import { ActionableClient, FolderMetadata, FuzzContext, HttpMethod, ItemId, Json, NoteData, RandomFolderOptions, UserData } from './types'; import { join } from 'path'; -import { mkdir } from 'fs-extra'; +import { mkdir, remove } from 'fs-extra'; import getStringProperty from './utils/getStringProperty'; import { strict as assert } from 'assert'; import ClipperServer from '@joplin/lib/ClipperServer'; import ActionTracker from './ActionTracker'; import Logger from '@joplin/utils/Logger'; -import execa = require('execa'); import { cliDirectory } from './constants'; import { commandToString } from '@joplin/utils'; import { quotePath } from '@joplin/utils/path'; import getNumberProperty from './utils/getNumberProperty'; import retryWithCount from './utils/retryWithCount'; +import { msleep, Second } from '@joplin/utils/time'; +import shim from '@joplin/lib/shim'; +import { spawn } from 'child_process'; +import AsyncActionQueue from '@joplin/lib/AsyncActionQueue'; +import { createInterface } from 'readline/promises'; +import Stream = require('stream'); const logger = Logger.create('Client'); +type OnCloseListener = ()=> void; + +type ChildProcessWrapper = { + stdout: Stream.Readable; + stderr: Stream.Readable; + writeStdin: (data: Buffer|string)=> void; + close: ()=> void; +}; + +// Should match the prompt used by the CLI "batch" command. +const cliProcessPromptString = 'command> '; + + class Client implements ActionableClient { public readonly email: string; @@ -61,9 +79,10 @@ class Client implements ActionableClient { profileDirectory, apiPort, apiToken, - closeAccount, ); + client.onClose(closeAccount); + // Joplin Server sync await client.execCliCommand_('config', 'sync.target', '9'); await client.execCliCommand_('config', 'sync.9.path', context.serverUrl); @@ -76,16 +95,7 @@ class Client implements ActionableClient { await client.execCliCommand_('e2ee', 'enable', '--password', e2eePassword); logger.info('Created and configured client'); - // Run asynchronously -- the API server command doesn't exit until the server - // is closed. - void (async () => { - try { - await client.execCliCommand_('server', 'start'); - } catch (error) { - logger.info('API server exited'); - logger.debug('API server exit status', error); - } - })(); + await client.startClipperServer_(); await client.sync(); return client; @@ -95,25 +105,112 @@ class Client implements ActionableClient { } } + private onCloseListeners_: OnCloseListener[] = []; + + private childProcess_: ChildProcessWrapper; + private childProcessQueue_ = new AsyncActionQueue(); + private bufferedChildProcessStdout_: string[] = []; + private bufferedChildProcessStderr_: string[] = []; + private onChildProcessOutput_: ()=> void = ()=>{}; + + private transcript_: string[] = []; + private constructor( private readonly tracker_: ActionableClient, userData: UserData, private readonly profileDirectory: string, private readonly apiPort_: number, private readonly apiToken_: string, - private readonly cleanUp_: ()=> Promise, ) { this.email = userData.email; + + // Don't skip child process-related tasks. + this.childProcessQueue_.setCanSkipTaskHandler(() => false); + + const initializeChildProcess = () => { + const rawChildProcess = spawn('yarn', [ + ...this.cliCommandArguments, + 'batch', + '--continue-on-failure', + '-', + ], { + cwd: cliDirectory, + }); + rawChildProcess.stdout.on('data', (chunk: Buffer) => { + const chunkString = chunk.toString('utf-8'); + this.transcript_.push(chunkString); + + this.bufferedChildProcessStdout_.push(chunkString); + this.onChildProcessOutput_(); + }); + rawChildProcess.stderr.on('data', (chunk: Buffer) => { + const chunkString = chunk.toString('utf-8'); + logger.warn('Child process', this.label, 'stderr:', chunkString); + + this.transcript_.push(chunkString); + this.bufferedChildProcessStderr_.push(chunkString); + this.onChildProcessOutput_(); + }); + + this.childProcess_ = { + writeStdin: (data: Buffer|string) => { + this.transcript_.push(data.toString()); + rawChildProcess.stdin.write(data); + }, + stderr: rawChildProcess.stderr, + stdout: rawChildProcess.stdout, + close: () => { + rawChildProcess.stdin.destroy(); + rawChildProcess.kill(); + }, + }; + }; + initializeChildProcess(); } + private async startClipperServer_() { + await this.execCliCommand_('server', '--quiet', '--exit-early', 'start'); + + // Wait for the server to start + await retryWithCount(async () => { + await this.execApiCommand_('GET', '/ping'); + }, { + count: 3, + onFail: async () => { + await msleep(1000); + }, + }); + } + + private closed_ = false; public async close() { - await this.execCliCommand_('server', 'stop'); - await this.cleanUp_(); + assert.ok(!this.closed_, 'should not be closed'); + + // Before removing the profile directory, verify that the profile directory is in the + // expected location: + const profileDirectory = this.profileDirectory; + assert.ok(profileDirectory, 'profile directory for client should be contained within the main temporary profiles directory (should be safe to delete)'); + await remove(profileDirectory); + + for (const listener of this.onCloseListeners_) { + listener(); + } + + this.childProcess_.close(); + this.closed_ = true; + } + + public onClose(listener: OnCloseListener) { + this.onCloseListeners_.push(listener); + } + + public get label() { + return this.email; } private get cliCommandArguments() { return [ - 'start-no-build', + 'start', '--profile', this.profileDirectory, '--env', 'dev', ]; @@ -121,47 +218,121 @@ class Client implements ActionableClient { public getHelpText() { return [ - `Client ${this.email}:`, + `Client ${this.label}:`, `\tCommand: cd ${quotePath(cliDirectory)} && ${commandToString('yarn', this.cliCommandArguments)}`, ].join('\n'); } + public getTranscript() { + const lines = this.transcript_.join('').split('\n'); + return ( + lines + // indent, for readability + .map(line => ` ${line}`) + // Since the server could still be running if the user posts the log, don't including + // web clipper tokens in the output: + .map(line => line.replace(/token=[a-z0-9A-Z_]+/g, 'token=*****')) + // Don't include the sync password in the output + .map(line => line.replace(/(config "(sync.9.password|api.token)") ".*"/, '$1 "****"')) + .join('\n') + ); + } + + // Connects the child process to the main terminal interface. + // Useful for debugging. + public async startCliDebugSession() { + this.childProcessQueue_.push(async () => { + this.onChildProcessOutput_ = () => { + process.stdout.write(this.bufferedChildProcessStdout_.join('\n')); + process.stderr.write(this.bufferedChildProcessStderr_.join('\n')); + this.bufferedChildProcessStdout_ = []; + this.bufferedChildProcessStderr_ = []; + }; + this.bufferedChildProcessStdout_ = []; + this.bufferedChildProcessStderr_ = []; + process.stdout.write('CLI debug session. Enter a blank line or "exit" to exit.\n'); + process.stdout.write('To review a transcript of all interactions with this client,\n'); + process.stdout.write('enter "[transcript]".\n\n'); + process.stdout.write(cliProcessPromptString); + + const isExitRequest = (input: string) => { + return input === 'exit' || input === ''; + }; + + // Per https://github.com/nodejs/node/issues/32291, we can't pipe process.stdin + // to childProcess_.stdin without causing issues. Forward using readline instead: + const readline = createInterface({ input: process.stdin, output: process.stdout }); + let lastInput = ''; + do { + lastInput = await readline.question(''); + if (lastInput === '[transcript]') { + process.stdout.write(`\n\n# Transcript\n\n${this.getTranscript()}\n\n# End transcript\n\n`); + } else if (!isExitRequest(lastInput)) { + this.childProcess_.writeStdin(`${lastInput}\n`); + } + } while (!isExitRequest(lastInput)); + + this.onChildProcessOutput_ = () => {}; + readline.close(); + }); + await this.childProcessQueue_.processAllNow(); + } + private async execCliCommand_(commandName: string, ...args: string[]) { assert.match(commandName, /^[a-z]/, 'Command name must start with a lowercase letter.'); - const commandResult = await execa('yarn', [ - ...this.cliCommandArguments, - commandName, - ...args, - ], { - cwd: cliDirectory, - // Connects /dev/null to stdin - stdin: 'ignore', + let commandStdout = ''; + let commandStderr = ''; + this.childProcessQueue_.push(() => { + return new Promise(resolve => { + this.onChildProcessOutput_ = () => { + const lines = this.bufferedChildProcessStdout_.join('\n').split('\n'); + const promptIndex = lines.lastIndexOf(cliProcessPromptString); + + if (promptIndex >= 0) { + commandStdout = lines.slice(0, promptIndex).join('\n'); + commandStderr = this.bufferedChildProcessStderr_.join('\n'); + + resolve(); + } else { + logger.debug('waiting...'); + } + }; + this.bufferedChildProcessStdout_ = []; + this.bufferedChildProcessStderr_ = []; + const command = `${[commandName, ...args.map(arg => JSON.stringify(arg))].join(' ')}\n`; + logger.debug('exec', command); + this.childProcess_.writeStdin(command); + }); }); - logger.debug('Ran command: ', commandResult.command, commandResult.exitCode); - logger.debug(' Output: ', commandResult.stdout); - return commandResult; + await this.childProcessQueue_.processAllNow(); + return { + stdout: commandStdout, + stderr: commandStderr, + }; } // eslint-disable-next-line no-dupe-class-members -- This is not a duplicate class member - private async execApiCommand_(method: 'GET', route: string): Promise; + private async execApiCommand_(method: 'GET', route: string): Promise; // eslint-disable-next-line no-dupe-class-members -- This is not a duplicate class member - private async execApiCommand_(method: 'POST'|'PUT', route: string, data: Json): Promise; + private async execApiCommand_(method: 'POST'|'PUT', route: string, data: Json): Promise; // eslint-disable-next-line no-dupe-class-members -- This is not a duplicate class member - private async execApiCommand_(method: HttpMethod, route: string, data: Json|null = null): Promise { + private async execApiCommand_(method: HttpMethod, route: string, data: Json|null = null): Promise { route = route.replace(/^[/]/, ''); const url = new URL(`http://localhost:${this.apiPort_}/${route}`); url.searchParams.append('token', this.apiToken_); - const response = await fetch(url, { + this.transcript_.push(`\n[[${method} ${url}; body: ${JSON.stringify(data)}]]\n`); + + const response = await shim.fetch(url.toString(), { method, - body: data ? JSON.stringify(data) : undefined, + ...(data ? { body: JSON.stringify(data) } : undefined), }); if (!response.ok) { throw new Error(`Request to ${route} failed with error: ${await response.text()}`); } - return await response.json(); + return await response.text(); } private async execPagedApiCommand_( @@ -177,9 +348,9 @@ class Client implements ActionableClient { for (let page = 1; hasMore; page++) { searchParams.set('page', String(page)); searchParams.set('limit', '10'); - const response = await this.execApiCommand_( + const response = JSON.parse(await this.execApiCommand_( method, `${route}?${searchParams}`, - ); + )); if ( typeof response !== 'object' || !('has_more' in response) @@ -199,38 +370,37 @@ class Client implements ActionableClient { } private async decrypt_() { - // E2EE decryption can occasionally fail with "Master key is not loaded:". - // Allow e2ee decryption to be retried: + const result = await this.execCliCommand_('e2ee', 'decrypt', '--force'); + if (!result.stdout.includes('Completed decryption.')) { + throw new Error(`Decryption did not complete: ${result.stdout}`); + } + } + + public async sync() { + logger.info('Sync', this.label); + await this.tracker_.sync(); + await retryWithCount(async () => { - const result = await this.execCliCommand_('e2ee', 'decrypt', '--force'); - if (!result.stdout.includes('Completed decryption.')) { - throw new Error(`Decryption did not complete: ${result.stdout}`); + const result = await this.execCliCommand_('sync'); + if (result.stdout.match(/Last error:/i)) { + throw new Error(`Sync failed: ${result.stdout}`); } + + await this.decrypt_(); }, { - count: 3, - onFail: async (error)=>{ - logger.warn('E2EE decryption failed:', error); - logger.info('Syncing before retry...'); - await this.execCliCommand_('sync'); + count: 4, + // Certain sync failures self-resolve after a background task is allowed to + // run. Delay: + delayOnFailure: retry => retry * Second * 2, + onFail: async (error) => { + logger.debug('Sync error: ', error); + logger.info('Sync failed. Retrying...'); }, }); } - public async sync() { - logger.info('Sync', this.email); - - await this.tracker_.sync(); - - const result = await this.execCliCommand_('sync'); - if (result.stdout.match(/Last error:/i)) { - throw new Error(`Sync failed: ${result.stdout}`); - } - - await this.decrypt_(); - } - public async createFolder(folder: FolderMetadata) { - logger.info('Create folder', folder.id, 'in', `${folder.parentId ?? 'root'}/${this.email}`); + logger.info('Create folder', folder.id, 'in', `${folder.parentId ?? 'root'}/${this.label}`); await this.tracker_.createFolder(folder); await this.execApiCommand_('POST', '/folders', { @@ -241,15 +411,27 @@ class Client implements ActionableClient { } private async assertNoteMatchesState_(expected: NoteData) { - assert.equal( - (await this.execCliCommand_('cat', expected.id)).stdout, - `${expected.title}\n\n${expected.body}`, - 'note should exist', - ); + await retryWithCount(async () => { + const noteContent = (await this.execCliCommand_('cat', expected.id)).stdout; + assert.equal( + // Compare without trailing newlines for consistency, the output from "cat" + // can sometimes have an extra newline (due to the CLI prompt) + noteContent.trimEnd(), + `${expected.title}\n\n${expected.body.trimEnd()}`, + 'note should exist', + ); + }, { + count: 3, + onFail: async () => { + // Send an event to the server and wait for it to be processed -- it's possible that the server + // hasn't finished processing the API event for creating the note: + await this.execApiCommand_('GET', '/ping'); + }, + }); } public async createNote(note: NoteData) { - logger.info('Create note', note.id, 'in', `${note.parentId}/${this.email}`); + logger.info('Create note', note.id, 'in', `${note.parentId}/${this.label}`); await this.tracker_.createNote(note); await this.execApiCommand_('POST', '/notes', { @@ -262,7 +444,7 @@ class Client implements ActionableClient { } public async updateNote(note: NoteData) { - logger.info('Update note', note.id, 'in', `${note.parentId}/${this.email}`); + logger.info('Update note', note.id, 'in', `${note.parentId}/${this.label}`); await this.tracker_.updateNote(note); await this.execApiCommand_('PUT', `/notes/${encodeURIComponent(note.id)}`, { title: note.title, @@ -273,7 +455,7 @@ class Client implements ActionableClient { } public async deleteFolder(id: string) { - logger.info('Delete folder', id, 'in', this.email); + logger.info('Delete folder', id, 'in', this.label); await this.tracker_.deleteFolder(id); await this.execCliCommand_('rmbook', '--permanent', '--force', id); @@ -282,8 +464,9 @@ class Client implements ActionableClient { public async shareFolder(id: string, shareWith: Client) { await this.tracker_.shareFolder(id, shareWith); - logger.info('Share', id, 'with', shareWith.email); + logger.info('Share', id, 'with', shareWith.label); await this.execCliCommand_('share', 'add', id, shareWith.email); + await this.sync(); await shareWith.sync(); @@ -307,6 +490,7 @@ class Client implements ActionableClient { ], 'there should be a single incoming share from the expected user'); await shareWith.execCliCommand_('share', 'accept', id); + await shareWith.sync(); } public async moveItem(itemId: ItemId, newParentId: ItemId) { @@ -329,7 +513,7 @@ class Client implements ActionableClient { item => ({ id: getStringProperty(item, 'id'), parentId: getNumberProperty(item, 'is_conflict') === 1 ? ( - `[conflicts for ${getStringProperty(item, 'conflict_original_id')} in ${this.email}]` + `[conflicts for ${getStringProperty(item, 'conflict_original_id')} in ${this.label}]` ) : getStringProperty(item, 'parent_id'), title: getStringProperty(item, 'title'), body: getStringProperty(item, 'body'), @@ -366,8 +550,8 @@ class Client implements ActionableClient { return this.tracker_.randomNote(); } - public async checkState(_allClients: Client[]) { - logger.info('Check state', this.email); + public async checkState() { + logger.info('Check state', this.label); type ItemSlice = { id: string }; const compare = (a: ItemSlice, b: ItemSlice) => { diff --git a/packages/tools/fuzzer/ClientPool.ts b/packages/tools/fuzzer/ClientPool.ts index 4e1d9fbcdb..65c40a5ab5 100644 --- a/packages/tools/fuzzer/ClientPool.ts +++ b/packages/tools/fuzzer/ClientPool.ts @@ -25,30 +25,44 @@ export default class ClientPool { } public constructor( private readonly context_: FuzzContext, - public readonly clients: Client[], - ) { } + private clients_: Client[], + ) { + for (const client of clients_) { + this.listenForClientClose_(client); + } + } + + private listenForClientClose_(client: Client) { + client.onClose(() => { + this.clients_ = this.clients_.filter(other => other !== client); + }); + } public randomClient(filter: ClientFilter = ()=>true) { - const clients = this.clients.filter(filter); + const clients = this.clients_.filter(filter); return clients[ this.context_.randInt(0, clients.length) ]; } public async checkState() { - for (const client of this.clients) { - await client.checkState(this.clients); + for (const client of this.clients_) { + await client.checkState(); } } public async syncAll() { - for (const client of this.clients) { + for (const client of this.clients_) { await client.sync(); } } + public get clients() { + return [...this.clients_]; + } + public helpText() { - return this.clients.map(client => client.getHelpText()).join('\n\n'); + return this.clients_.map(client => client.getHelpText()).join('\n\n'); } } diff --git a/packages/tools/fuzzer/sync-fuzzer.ts b/packages/tools/fuzzer/sync-fuzzer.ts index 1b67c94569..1b4b6a3974 100644 --- a/packages/tools/fuzzer/sync-fuzzer.ts +++ b/packages/tools/fuzzer/sync-fuzzer.ts @@ -3,7 +3,6 @@ import { join } from 'path'; import { exists, mkdir, remove } from 'fs-extra'; import Setting, { Env } from '@joplin/lib/models/Setting'; import Logger, { TargetType } from '@joplin/utils/Logger'; -import { waitForCliInput } from '@joplin/utils/cli'; import Server from './Server'; import { CleanupTask, FuzzContext } from './types'; import ClientPool from './ClientPool'; @@ -13,6 +12,8 @@ import SeededRandom from './utils/SeededRandom'; import { env } from 'process'; import yargs = require('yargs'); import { strict as assert } from 'assert'; +import openDebugSession from './utils/openDebugSession'; +import { Second } from '@joplin/utils/time'; const { shimInit } = require('@joplin/lib/shim-init-node'); const globalLogger = new Logger(); @@ -237,7 +238,7 @@ const main = async (options: Options) => { process.exit(1); }); - let clientHelpText; + let clientPool: ClientPool|null = null; try { const joplinServerUrl = 'http://localhost:22300/'; @@ -263,12 +264,12 @@ const main = async (options: Options) => { execApi: server.execApi.bind(server), randInt: (a, b) => random.nextInRange(a, b), }; - const clientPool = await ClientPool.create( + clientPool = await ClientPool.create( fuzzContext, options.clientCount, task => { cleanupTasks.push(task); }, ); - clientHelpText = clientPool.helpText(); + await clientPool.syncAll(); const maxSteps = options.maximumSteps; for (let stepIndex = 1; maxSteps <= 0 || stepIndex <= maxSteps; stepIndex++) { @@ -293,7 +294,8 @@ const main = async (options: Options) => { await retryWithCount(async () => { await clientPool.checkState(); }, { - count: 3, + count: 4, + delayOnFailure: count => count * Second * 2, onFail: async () => { logger.info('.checkState failed. Syncing all clients...'); await clientPool.syncAll(); @@ -302,11 +304,10 @@ const main = async (options: Options) => { } } catch (error) { logger.error('ERROR', error); - if (clientHelpText) { - logger.info('Client information:\n', clientHelpText); + if (clientPool) { + logger.info('Client information:\n', clientPool.helpText()); + await openDebugSession(clientPool); } - logger.info('An error occurred. Pausing before continuing cleanup.'); - await waitForCliInput(); process.exitCode = 1; } finally { await cleanUp(); diff --git a/packages/tools/fuzzer/utils/openDebugSession.ts b/packages/tools/fuzzer/utils/openDebugSession.ts new file mode 100644 index 0000000000..66eba72f1d --- /dev/null +++ b/packages/tools/fuzzer/utils/openDebugSession.ts @@ -0,0 +1,50 @@ +import Logger from '@joplin/utils/Logger'; +import ClientPool from '../ClientPool'; +import { createInterface } from 'readline/promises'; + +const logger = Logger.create('openDebugSession'); + +const openDebugSession = async (clients: ClientPool) => { + const allClients = clients.clients; + const clientChoices = allClients.map((client, index) => { + return `${index}: ${client.label}`; + }).join('\n'); + + const askForClient = async (questionPrefix = '') => { + // Recreate the readline interface each time to avoid conflicting + // with the debug sessions for the individual clients. + const readlineInterface = createInterface({ + input: process.stdin, + output: process.stdout, + }); + try { + const clientChoice = await readlineInterface.question( + `${questionPrefix}Select a client from:\n${clientChoices}\nclient: `, + ); + if (clientChoice.trim() === '' || clientChoice === 'exit') { + return null; + } + + const asNumber = Number(clientChoice); + if (!isFinite(asNumber) || Math.floor(asNumber) !== asNumber) { + return askForClient('Please input an integer. '); + } + + if (asNumber < 0 || asNumber >= allClients.length) { + return askForClient('Choice out of range. '); + } + + return allClients[asNumber]; + } finally { + readlineInterface.close(); + } + }; + + for (let client = await askForClient(); client; client = await askForClient()) { + logger.info('Switching to client', client.getHelpText()); + + await client.startCliDebugSession(); + } +}; + +export default openDebugSession; diff --git a/packages/tools/fuzzer/utils/retryWithCount.ts b/packages/tools/fuzzer/utils/retryWithCount.ts index ce89a10eb4..760a9afa9e 100644 --- a/packages/tools/fuzzer/utils/retryWithCount.ts +++ b/packages/tools/fuzzer/utils/retryWithCount.ts @@ -1,9 +1,15 @@ +import Logger from '@joplin/utils/Logger'; +import { msleep } from '@joplin/utils/time'; + +const logger = Logger.create('retryWithCount'); + interface Options { count: number; + delayOnFailure?: (retryCount: number)=> number; onFail: (error: Error)=> Promise; } -const retryWithCount = async (task: ()=> Promise, { count, onFail }: Options) => { +const retryWithCount = async (task: ()=> Promise, { count, delayOnFailure, onFail }: Options) => { let lastError: Error|null = null; for (let retry = 0; retry < count; retry ++) { try { @@ -11,6 +17,13 @@ const retryWithCount = async (task: ()=> Promise, { count, onFail }: Optio } catch (error) { await onFail(error); lastError = error; + + const willRetry = retry + 1 < count; + const delay = willRetry && delayOnFailure ? delayOnFailure(retry + 1) : 0; + if (delay) { + logger.info(`Retrying after ${delay}ms...`); + await msleep(delay); + } } }