diff --git a/.changeset/review-fixes-v0411.md b/.changeset/review-fixes-v0411.md new file mode 100644 index 0000000..af9ae68 --- /dev/null +++ b/.changeset/review-fixes-v0411.md @@ -0,0 +1,5 @@ +--- +'@offckb/cli': patch +--- + +Fix review findings from the v0.4.11 release merge. `offckb logs script` now scans a wider window before filtering so sparse script entries are not missed, and an unknown `offckb logs` target is rejected instead of silently reading the node log. `offckb logs -f` detects log rotation by inode change, so a rotated-in file that is already larger than the old one is re-read from the start. `offckb logs --tail` rejects zero and negative values instead of printing the entire filtered log. CKB 0.205.0 prerelease binaries (rc builds), including prereleases of newer versions, are now correctly treated as Terminal-RPC capable during chain init. A ckb-tui binary that lost its execute bit is reinstalled instead of failing at spawn time. The RPC proxy keeps appending events when a proxy.log rollover fails (previously all further events were dropped) and preserves the previous archive when the rollover's rename fails, records batched JSON-RPC requests including batched `send_transaction`, skips malformed batch members without aborting the rest of the batch, and warns instead of misreporting a parse error when `send_transaction` has no usable params. `--verbose` on `offckb node` now also enables the proxy's per-request debug lines. diff --git a/src/cli.ts b/src/cli.ts index 5015a66..91f1d50 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -111,8 +111,8 @@ program .option('--grep ', 'Only show lines containing the given text') .option('--tail ', 'Show the last N lines before following', (value: string) => { const parsed = Number(value); - if (!Number.isInteger(parsed) || parsed < 0) { - throw new InvalidArgumentError('--tail must be a non-negative integer'); + if (!Number.isInteger(parsed) || parsed <= 0) { + throw new InvalidArgumentError('--tail must be a positive integer'); } return parsed; }) diff --git a/src/cmd/logs.ts b/src/cmd/logs.ts index 09e0530..1885e07 100644 --- a/src/cmd/logs.ts +++ b/src/cmd/logs.ts @@ -19,6 +19,10 @@ export interface LogsOptions { } const DEFAULT_TAIL = 100; +// Script entries are sparse in run.log (a devnet node writes many non-script +// lines per second), so a script-filtered tail scans a much wider window and +// trims back to `tail` after filtering. +const SCRIPT_SCAN_FACTOR = 100; /** * Print (and optionally follow) a devnet log file. The core is synchronous so @@ -28,10 +32,16 @@ const DEFAULT_TAIL = 100; export function showLogs(target: LogTarget, options: LogsOptions, settings: Settings, logger: UnifiedLogger): void { const filePath = resolveLogPath(target, settings); const tail = options.tail ?? DEFAULT_TAIL; - - let lines = readLogTail(filePath, tail); + // A zero tail is rejected, not "print nothing": slice(-0) is slice(0), so + // script mode would otherwise dump every filtered line it scanned. + if (!Number.isInteger(tail) || tail <= 0) { + throw new Error(`--tail must be a positive integer (got ${options.tail})`); + } const scriptOnly = target === 'script'; - if (scriptOnly) lines = filterLinesByTarget(lines, SCRIPT_LOG_TARGET); + + const scanWindow = scriptOnly ? tail * SCRIPT_SCAN_FACTOR : tail; + let lines = readLogTail(filePath, scanWindow); + if (scriptOnly) lines = filterLinesByTarget(lines, SCRIPT_LOG_TARGET).slice(-tail); if (options.grep) lines = grepLines(lines, options.grep); for (const line of lines) logger.info(line); @@ -52,6 +62,9 @@ export function showLogs(target: LogTarget, options: LogsOptions, settings: Sett } export function logsCommand(target: string | undefined, options: LogsOptions): void { - const resolved: LogTarget = LOG_TARGETS.includes(target as LogTarget) ? (target as LogTarget) : 'node'; + if (target != null && !LOG_TARGETS.includes(target as LogTarget)) { + throw new Error(`Unknown log target '${target}'. Use one of: ${LOG_TARGETS.join(', ')}.`); + } + const resolved: LogTarget = (target as LogTarget) ?? 'node'; showLogs(resolved, options, readSettings(), defaultLogger); } diff --git a/src/cmd/node.ts b/src/cmd/node.ts index 23882bb..3f743fd 100644 --- a/src/cmd/node.ts +++ b/src/cmd/node.ts @@ -203,7 +203,7 @@ export async function nodeDevnet({ version, binaryPath, daemon, verbose }: NodeP throw new Error('CKB devnet exited while the miner was starting.'); } - const proxy = createRPCProxy(Network.devnet, settings.devnet.rpcUrl, settings.devnet.rpcProxyPort); + const proxy = createRPCProxy(Network.devnet, settings.devnet.rpcUrl, settings.devnet.rpcProxyPort, { verbose }); proxy.start(); // Contract script debug output (debug! in scripts) streams live over the diff --git a/src/devnet/log-file.ts b/src/devnet/log-file.ts index b5bef6e..ca8a78e 100644 --- a/src/devnet/log-file.ts +++ b/src/devnet/log-file.ts @@ -122,7 +122,10 @@ export function followLogFile(filePath: string, onLine: (line: string) => void, let decoder = new TextDecoder('utf-8'); const onChange = (curr: fs.Stats, prev: fs.Stats) => { - if (curr.size < prev.size || curr.size < offset) { + // Rotation swaps in a new inode at the same path; a replacement that is + // already larger than the old file fools the size checks below, so the + // inode change must also reset the offset. + if (curr.size < prev.size || curr.size < offset || curr.ino !== prev.ino) { // Truncated or rotated: restart from the beginning. offset = 0; partial = ''; diff --git a/src/node/init-chain.ts b/src/node/init-chain.ts index 94f9534..967ffc0 100644 --- a/src/node/init-chain.ts +++ b/src/node/init-chain.ts @@ -70,10 +70,14 @@ export const TERMINAL_RPC_MIN_CKB_VERSION = '0.205.0'; // Unknown (null/undefined/unparseable) versions keep the historical behavior // of assuming support — a custom binary whose version cannot be probed must -// not lose functionality it may actually have. +// not lose functionality it may actually have. The `-0` range suffix opts +// 0.205.0 prereleases (rc builds already carry the Terminal module) into the +// match; plain semver.gte would exclude them. includePrerelease extends that +// to prereleases of NEWER versions (0.205.1-rc1, 0.206.0-rc1, ...), which the +// tuple rule would otherwise reject even though they are past the minimum. export function supportsTerminalRpcModule(ckbVersion: string | null | undefined): boolean { if (ckbVersion == null || !semver.valid(ckbVersion)) return true; - return semver.gte(ckbVersion, TERMINAL_RPC_MIN_CKB_VERSION); + return semver.satisfies(ckbVersion, `>=${TERMINAL_RPC_MIN_CKB_VERSION}-0`, { includePrerelease: true }); } // Whether the chain's ckb.toml currently enables the Terminal RPC module. diff --git a/src/tools/ckb-tui.ts b/src/tools/ckb-tui.ts index f04b33c..b761b85 100644 --- a/src/tools/ckb-tui.ts +++ b/src/tools/ckb-tui.ts @@ -127,6 +127,11 @@ export class CKBTui { // Metadata alone does not prove readability; an unreadable binary must // flow into the reinstall path (which republishes with correct modes). fs.accessSync(binaryPath, fs.constants.R_OK); + // The binary is spawned directly, so a lost execute bit must also count + // as a mismatch. Windows reports X_OK for any readable file. + if (process.platform !== 'win32') { + fs.accessSync(binaryPath, fs.constants.X_OK); + } if (!expected) { return true; } diff --git a/src/tools/proxy-events.ts b/src/tools/proxy-events.ts index d84f715..520fe75 100644 --- a/src/tools/proxy-events.ts +++ b/src/tools/proxy-events.ts @@ -63,9 +63,28 @@ export function createProxyEventLog(filePath: string, maxBytes = PROXY_LOG_MAX_B const line = `${new Date().toISOString()} ${sanitizeEventText(text)}\n`; if (size < 0) size = fs.existsSync(filePath) ? fs.statSync(filePath).size : 0; if (size > 0 && size + Buffer.byteLength(line) > maxBytes) { - fs.rmSync(`${filePath}.1`, { force: true }); - fs.renameSync(filePath, `${filePath}.1`); - size = 0; + try { + // The previous archive is moved aside rather than deleted up + // front: if the active-log rename then fails, the archive is + // restored instead of permanently lost. + const archivePath = `${filePath}.1`; + const backupPath = `${filePath}.1.bak`; + fs.rmSync(backupPath, { force: true }); + const hadArchive = fs.existsSync(archivePath); + if (hadArchive) fs.renameSync(archivePath, backupPath); + try { + fs.renameSync(filePath, archivePath); + } catch (error) { + if (hadArchive) fs.renameSync(backupPath, archivePath); + throw error; + } + fs.rmSync(backupPath, { force: true }); + size = 0; + } catch { + // Rollover failed (for example the file is locked). Keep + // appending so events are not lost, and retry on the next event. + size = maxBytes; + } } fs.appendFileSync(filePath, line); size += Buffer.byteLength(line); @@ -87,33 +106,52 @@ export function handleProxyRequestBody(reqData: string, ctx: ProxyEventContext): if (reqData.length === 0) return; try { - const jsonRpcContent = JSON.parse(reqData) as JsonRpcRequestPayload; - const method = jsonRpcContent.method; - const params = jsonRpcContent.params; - ctx.sink.debug('RPC Req: ', method); - if (typeof method === 'string') { - ctx.events.event(`request ${method}`); - } - - if (method === 'send_transaction') { - const tx = (params as unknown[])[0]; - const txHash = ctx.hashTransaction(tx); - if (!fs.existsSync(ctx.transactionsPath)) { - fs.mkdirSync(ctx.transactionsPath, { recursive: true }); + // A JSON-RPC batch request is an array of payloads; normalize so batched + // send_transaction calls are recorded like single ones. + const parsed = JSON.parse(reqData) as JsonRpcRequestPayload | JsonRpcRequestPayload[]; + for (const jsonRpcContent of Array.isArray(parsed) ? parsed : [parsed]) { + // Batch members are user input: JSON.parse happily yields null, strings, + // or nested arrays. Skip anything that is not a plain object so one + // malformed member cannot abort the rest of the batch. + if (jsonRpcContent == null || typeof jsonRpcContent !== 'object' || Array.isArray(jsonRpcContent)) { + ctx.sink.warn('skipping malformed JSON-RPC batch member'); + continue; } - const txFile = path.resolve(ctx.transactionsPath, `${txHash}.json`); - fs.writeFileSync(txFile, JSON.stringify(tx, null, 2)); - // The hash line mirrors the RPC method name on purpose: at request time - // the proxy does not yet know whether the node will accept the tx, and - // any rejection surfaces separately as an RPC error line. - ctx.sink.info(`send_transaction: ${txHash}`); - ctx.events.event(`send_transaction ${txHash}`); + handleOneRequest(jsonRpcContent, ctx); } } catch (err) { ctx.sink.error('Error parsing JSON-RPC req content:', (err as Error).message); } } +function handleOneRequest(jsonRpcContent: JsonRpcRequestPayload, ctx: ProxyEventContext): void { + const method = jsonRpcContent.method; + const params = jsonRpcContent.params; + ctx.sink.debug('RPC Req: ', method); + if (typeof method === 'string') { + ctx.events.event(`request ${method}`); + } + + if (method === 'send_transaction') { + if (!Array.isArray(params) || params.length === 0) { + ctx.sink.warn('send_transaction request has no params; skipping tx dump'); + return; + } + const tx = params[0]; + const txHash = ctx.hashTransaction(tx); + if (!fs.existsSync(ctx.transactionsPath)) { + fs.mkdirSync(ctx.transactionsPath, { recursive: true }); + } + const txFile = path.resolve(ctx.transactionsPath, `${txHash}.json`); + fs.writeFileSync(txFile, JSON.stringify(tx, null, 2)); + // The hash line mirrors the RPC method name on purpose: at request time + // the proxy does not yet know whether the node will accept the tx, and + // any rejection surfaces separately as an RPC error line. + ctx.sink.info(`send_transaction: ${txHash}`); + ctx.events.event(`send_transaction ${txHash}`); + } +} + interface JsonRpcErrorPayload { error?: { code?: unknown; message?: unknown }; } diff --git a/src/tools/rpc-proxy.ts b/src/tools/rpc-proxy.ts index 50304fc..2b73c75 100644 --- a/src/tools/rpc-proxy.ts +++ b/src/tools/rpc-proxy.ts @@ -2,18 +2,32 @@ import httpProxy from 'http-proxy'; import http from 'http'; import { Network } from '../type/base'; import { readSettings } from '../cfg/setting'; -import { logger } from '../util/logger'; +import { logger, UnifiedLogger } from '../util/logger'; import { proxyLogPathForNetwork } from '../devnet/log-file'; -import { createProxyEventLog, handleProxyRequestBody, handleProxyResponseBody } from './proxy-events'; +import { + createProxyEventLog, + handleProxyRequestBody, + handleProxyResponseBody, + ProxyEventContext, +} from './proxy-events'; // todo: if we use import this throws error in tsc building const { cccA } = require('@ckb-ccc/core/advanced'); -export function createRPCProxy(network: Network, targetRpcUrl: string, port: number) { +export interface RPCProxyOptions { + // --verbose also lifts the proxy's per-request "RPC Req" lines from debug + // to the console; without it they stay in proxy.log only. + verbose?: boolean; +} + +export function createRPCProxy(network: Network, targetRpcUrl: string, port: number, options: RPCProxyOptions = {}) { const settings = readSettings(); const events = createProxyEventLog(proxyLogPathForNetwork(network, settings)); - const ctx = { - sink: logger, + // The global logger sits at info level, so a debug-level sink is needed for + // verbose runs to actually see the per-request lines. + const sink = options.verbose ? UnifiedLogger.create({ level: 'debug', showLevel: false }) : logger; + const ctx: ProxyEventContext = { + sink, events, transactionsPath: settings[network].transactionsPath, hashTransaction: (tx: unknown) => { diff --git a/tests/ckb-tui-install.test.ts b/tests/ckb-tui-install.test.ts index 4b308c5..8e9e847 100644 --- a/tests/ckb-tui-install.test.ts +++ b/tests/ckb-tui-install.test.ts @@ -97,7 +97,9 @@ describe('ckb-tui installed-binary verification', () => { )[assetName()]; it('keeps an existing binary whose digest matches the configured release', () => { - fs.writeFileSync(binaryPath, 'installed ckb-tui'); + // A real install publishes with mode 0o755; the execute bit is part of + // what the verification checks. + fs.writeFileSync(binaryPath, 'installed ckb-tui', { mode: 0o755 }); jest.spyOn(crypto, 'createHash').mockReturnValue({ update: () => ({ digest: () => pinnedDigest() }), } as unknown as crypto.Hash); @@ -145,7 +147,8 @@ describe('ckb-tui installed-binary verification', () => { it('falls back to presence-only detection for a release without a pinned binary digest', () => { mockVersion.current = 'v9.9.9'; - fs.writeFileSync(binaryPath, 'any content at all'); + // Presence-only still means a usable binary: regular, readable, executable. + fs.writeFileSync(binaryPath, 'any content at all', { mode: 0o755 }); expect(CKBTui.ensureInstalled()).toBe(binaryPath); expect(installSpy).not.toHaveBeenCalled(); @@ -181,6 +184,14 @@ describe('ckb-tui installed-binary verification', () => { expect(internals.installedBinaryMatches(binaryPath)).toBe(false); }); + + itPosix('treats a binary without the execute bit as a mismatch', () => { + mockVersion.current = 'v9.9.9'; // Unpinned: presence alone must not suffice. + // No execute bit anywhere, so X_OK fails even for root. + fs.writeFileSync(binaryPath, 'execute-bit-lost ckb-tui', { mode: 0o644 }); + + expect(internals.installedBinaryMatches(binaryPath)).toBe(false); + }); }); describe('ckb-tui binary publishing', () => { diff --git a/tests/init-chain.test.ts b/tests/init-chain.test.ts index d8e7b63..9e8c154 100644 --- a/tests/init-chain.test.ts +++ b/tests/init-chain.test.ts @@ -223,7 +223,12 @@ describe('Terminal RPC module version gating', () => { expect(supportsTerminalRpcModule('0.207.0')).toBe(true); expect(supportsTerminalRpcModule('0.120.0')).toBe(false); expect(supportsTerminalRpcModule('0.204.9')).toBe(false); - expect(supportsTerminalRpcModule('0.205.0-rc1')).toBe(false); + // 0.205.0 prereleases already ship the Terminal module. + expect(supportsTerminalRpcModule('0.205.0-rc1')).toBe(true); + expect(supportsTerminalRpcModule('0.204.9-rc1')).toBe(false); + // Prereleases of newer versions are past the minimum too. + expect(supportsTerminalRpcModule('0.205.1-rc1')).toBe(true); + expect(supportsTerminalRpcModule('0.206.0-rc1')).toBe(true); }); it('strips Terminal from a freshly initialized template when the binary is too old', async () => { diff --git a/tests/logs-command.test.ts b/tests/logs-command.test.ts index e1e308d..3c8baf1 100644 --- a/tests/logs-command.test.ts +++ b/tests/logs-command.test.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import winston from 'winston'; -import { showLogs } from '../src/cmd/logs'; +import { logsCommand, showLogs } from '../src/cmd/logs'; import { defaultSettings, Settings } from '../src/cfg/setting'; import { UnifiedLogger } from '../src/util/logger'; @@ -70,6 +70,17 @@ describe('showLogs', () => { expect(transport.logs).toEqual([SCRIPT_LINE]); }); + it('finds sparse script entries beyond the raw tail window', () => { + const { settings, transport } = fixture(); + const runLog = path.join(settings.devnet.dataPath, 'logs', 'run.log'); + // The only script entry sits above the last `tail` lines, so a plain + // filter-after-tail would print nothing. + fs.writeFileSync(runLog, [SCRIPT_LINE, NODE_LINE, ERROR_LINE, NODE_LINE].join('\n') + '\n'); + const log = UnifiedLogger.create({ transports: [transport], showLevel: false }); + showLogs('script', { tail: 2 }, settings, log); + expect(transport.logs).toEqual([SCRIPT_LINE]); + }); + it('reads miner.log for the miner target', () => { const { settings, transport } = fixture(); const log = UnifiedLogger.create({ transports: [transport], showLevel: false }); @@ -91,13 +102,23 @@ describe('showLogs', () => { expect(transport.logs).toEqual([ERROR_LINE]); }); + it('rejects a zero or negative tail instead of dumping the whole filtered log', () => { + const { settings, transport } = fixture(); + const log = UnifiedLogger.create({ transports: [transport], showLevel: false }); + // slice(-0) is slice(0): without validation, script mode would print every + // filtered line it scanned instead of nothing. + expect(() => showLogs('script', { tail: 0 }, settings, log)).toThrow(/positive integer/); + expect(() => showLogs('script', { tail: -0 }, settings, log)).toThrow(/positive integer/); + expect(() => showLogs('node', { tail: -5 }, settings, log)).toThrow(/positive integer/); + expect(transport.logs).toEqual([]); + }); + it('throws a helpful error when the log file does not exist', () => { const settings = JSON.parse(JSON.stringify(defaultSettings)) as Settings; settings.devnet.dataPath = '/nonexistent'; const log = UnifiedLogger.create({ transports: [new CapturingTransport()] }); expect(() => showLogs('node', { tail: 100 }, settings, log)).toThrow(/log file not found/i); }); - it('in follow mode gates script entries and applies grep to streamed lines', () => { const { settings, transport } = fixture(); type StatListener = (curr: fs.Stats, prev: fs.Stats) => void; @@ -131,3 +152,10 @@ describe('showLogs', () => { } }); }); + +describe('logsCommand', () => { + it('rejects an unknown log target instead of falling back to node', () => { + expect(() => logsCommand('scrpit', {})).toThrow(/unknown log target 'scrpit'/i); + expect(() => logsCommand('scrpit', {})).toThrow(/node, script, miner, rpc/); + }); +}); diff --git a/tests/logs.test.ts b/tests/logs.test.ts index ccf90a0..8bfc78f 100644 --- a/tests/logs.test.ts +++ b/tests/logs.test.ts @@ -194,11 +194,25 @@ describe('followLogFile', () => { listeners[0](grown, grown); expect(seen).toEqual([SCRIPT_LINE]); - // Rotation replaces the file with a shorter one; the next notification - // must reset the offset instead of seeking past the end. + // Truncation shrinks the file in place; the next notification must + // reset the offset instead of seeking past the end. fs.writeFileSync(file, `${ERROR_LINE}\n`); listeners[0](fs.statSync(file), grown); expect(seen).toEqual([SCRIPT_LINE, ERROR_LINE]); + + fs.appendFileSync(file, `${NODE_LINE}\n`); + const regrown = fs.statSync(file); + listeners[0](regrown, regrown); + expect(seen).toEqual([SCRIPT_LINE, ERROR_LINE, NODE_LINE]); + + // Rotation replaces the file: the old path is renamed away and a new + // file appears at the same path, possibly already larger than the old + // one. The size checks alone would keep the stale offset, so the inode + // change must restart the read from offset 0. + fs.renameSync(file, `${file}.1`); + fs.writeFileSync(file, [SCRIPT_LINE, ERROR_LINE, NODE_LINE, SCRIPT_LINE].join('\n') + '\n'); + listeners[0](fs.statSync(file), regrown); + expect(seen).toEqual([SCRIPT_LINE, ERROR_LINE, NODE_LINE, SCRIPT_LINE, ERROR_LINE, NODE_LINE, SCRIPT_LINE]); stop(); } finally { mockWatchFile.mockReset(); diff --git a/tests/proxy-events.test.ts b/tests/proxy-events.test.ts index e5e9f6a..cd46bb9 100644 --- a/tests/proxy-events.test.ts +++ b/tests/proxy-events.test.ts @@ -3,6 +3,20 @@ import * as os from 'os'; import * as path from 'path'; import { createProxyEventLog, handleProxyRequestBody, handleProxyResponseBody } from '../src/tools/proxy-events'; +// renameSync goes through a mock so rollover failures can be simulated +// deterministically (same pattern as the fs.watchFile stub in +// logs-command.test.ts); the default delegates to the real implementation. +const mockRenameSync = jest.fn(); +jest.mock('fs', () => ({ + ...jest.requireActual('fs'), + renameSync: (...args: unknown[]) => mockRenameSync(...args), +})); + +const realFs = jest.requireActual('fs') as typeof fs; +beforeEach(() => { + mockRenameSync.mockImplementation(realFs.renameSync); +}); + function makeSink() { const sink = { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn() }; return sink; @@ -50,6 +64,49 @@ describe('handleProxyRequestBody', () => { expect(content).toMatch(/send_transaction 0xhash/); }); + it('records each entry of a batch request, including a batched send_transaction', () => { + const ctx = makeCtx(transactionsPath); + const tx = { cell_deps: [], inputs: [], outputs: [] }; + handleProxyRequestBody( + JSON.stringify([ + { jsonrpc: '2.0', id: 1, method: 'get_tip_header', params: [] }, + { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [tx] }, + ]), + ctx, + ); + expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash')); + expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true); + const content = fs.readFileSync(ctx.events.filePath, 'utf8'); + expect(content).toMatch(/request get_tip_header/); + expect(content).toMatch(/send_transaction 0xhash/); + }); + + it('skips malformed batch members and still records the requests after them', () => { + const ctx = makeCtx(transactionsPath); + const tx = { cell_deps: [], inputs: [], outputs: [] }; + handleProxyRequestBody( + JSON.stringify([null, { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [tx] }]), + ctx, + ); + // The null member is skipped with a warning rather than aborting the batch. + expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('malformed')); + expect(ctx.sink.error).not.toHaveBeenCalled(); + // The valid send_transaction after it is still recorded. + expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash')); + expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true); + const content = fs.readFileSync(ctx.events.filePath, 'utf8'); + expect(content).toMatch(/send_transaction 0xhash/); + }); + + it('warns and skips the tx dump when send_transaction has no usable params', () => { + const ctx = makeCtx(transactionsPath); + handleProxyRequestBody(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'send_transaction' }), ctx); + expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('no params')); + // A missing-params request is not a parse failure. + expect(ctx.sink.error).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(false); + }); + it('reports malformed JSON-RPC bodies at error level', () => { const ctx = makeCtx(transactionsPath); handleProxyRequestBody('not json', ctx); @@ -161,4 +218,57 @@ describe('createProxyEventLog', () => { expect(current).toMatch(/get_tip_header/); expect(current).not.toMatch(/get_blockchain_info/); }); + + it('keeps appending when a rollover fails and retries on the next event', () => { + const file = path.join(dir, 'logs', 'proxy.log'); + const log = createProxyEventLog(file, 60); + log.event('request get_tip_header'); + log.event('request get_blockchain_info'); + + // Block the rollover: every rename fails, so the active log stays put. + mockRenameSync.mockImplementation(() => { + throw new Error('rename blocked by test'); + }); + log.event('request get_tip_header'); + mockRenameSync.mockImplementation(realFs.renameSync); + + // The event is appended to the live file instead of being dropped. + const current = fs.readFileSync(file, 'utf8'); + expect(current).toMatch(/get_blockchain_info/); + expect(current).toMatch(/get_tip_header/); + + // Once the blockage is gone the next event rolls over normally. + log.event('request get_tip_header'); + const rolled = fs.readFileSync(`${file}.1`, 'utf8'); + expect(rolled).toMatch(/get_blockchain_info/); + expect(rolled).toMatch(/get_tip_header/); + }); + + it('preserves the previous archive when the active-log rename fails', () => { + const file = path.join(dir, 'logs', 'proxy.log'); + const log = createProxyEventLog(file, 60); + log.event('request get_blockchain_info'); + // Second event rolls the log: .1 now holds the first generation. + log.event('request get_tip_header'); + const archiveBefore = fs.readFileSync(`${file}.1`, 'utf8'); + expect(archiveBefore).toMatch(/get_blockchain_info/); + + // Fail only the active-log rename; the archive is moved aside first and + // must be put back when the swap cannot complete. + mockRenameSync.mockImplementation((oldPath: fs.PathLike, newPath: fs.PathLike) => { + if (String(oldPath) === file && String(newPath) === `${file}.1`) { + throw new Error('active-log rename blocked by test'); + } + return realFs.renameSync(oldPath, newPath); + }); + log.event('request get_tip_block_hash'); + + // The prior archive survived intact and no staging file was left behind. + expect(fs.readFileSync(`${file}.1`, 'utf8')).toBe(archiveBefore); + expect(fs.existsSync(`${file}.1.bak`)).toBe(false); + // The event that triggered the failed rollover was appended, not dropped. + const current = fs.readFileSync(file, 'utf8'); + expect(current).toMatch(/get_tip_header/); + expect(current).toMatch(/get_tip_block_hash/); + }); });