diff --git a/changelog.d/2379-clear-enoent-quiet.fixed.md b/changelog.d/2379-clear-enoent-quiet.fixed.md new file mode 100644 index 00000000..5f740206 --- /dev/null +++ b/changelog.d/2379-clear-enoent-quiet.fixed.md @@ -0,0 +1 @@ +Settings → Feedback's **Clear** no longer writes `[ds-bot] diagnostics log not truncated: ENOENT` when there is no log file on disk yet: a home whose `logs/` folder does not exist has nothing to empty, so the Clear stays silent on stderr and the window's `Cleared the error log.` is what happened. A leaf that is there and refuses the truncate (`0444`, `EACCES`) still names the code on exactly one line, and the answer for it is still `leaf: 'kept'` ([#2379](https://github.com/ThinkFlowLab/FlyBot/issues/2379)). diff --git a/core/src/host/diagnostics.js b/core/src/host/diagnostics.js index 58794dfd..d70211cc 100644 --- a/core/src/host/diagnostics.js +++ b/core/src/host/diagnostics.js @@ -308,11 +308,17 @@ export function install(rt) { await writeFile(logPath, '', 'utf8') result.leaf = 'truncated' } catch (error) { - result.leaf = error?.code === 'ENOENT' ? 'absent' : 'kept' - // The answer alone is not enough: it reaches the window, and the host log said nothing about - // a leaf left full. `append` names this same class of failure on one line, so clear does too; - // the line carries the code and never the folder, and the throw stays so the chain ends here. - console.error('[ds-bot] diagnostics log not truncated:', error?.code ?? clean(error?.message ?? String(error))) + // `ENOENT` is not a refusal: there was no leaf on disk to empty, so there was nothing to + // truncate — the same answer the `!toFile` host above gives, and it says so on no line at + // all. Every other code is a leaf we wanted to empty and could not: the window hears + // `kept`, and the host log names the code on exactly one line (#2364), never the folder. + // The throw stays so the chain ends here either way. + if (error?.code === 'ENOENT') { + result.leaf = 'absent' + } else { + result.leaf = 'kept' + console.error('[ds-bot] diagnostics log not truncated:', error?.code ?? clean(error?.message ?? String(error))) + } throw error } // Rotation moves the full log to `.1`, and the truncate alone left every rotated line diff --git a/core/test/diagnostics.test.js b/core/test/diagnostics.test.js index d688c652..795c62cc 100644 --- a/core/test/diagnostics.test.js +++ b/core/test/diagnostics.test.js @@ -954,3 +954,48 @@ test('#2364: a clear that cannot truncate leaves the readable surface alone', as assert.deepEqual(await rt.diagnostics.entries(), [], 'a real clear lists nothing afterwards') assert.equal(await rt.diagnostics.logText(), '', 'a real clear leaves nothing to download') }) + +// A Clear on a home with no log folder used to print "not truncated" for a write that failed +// with ENOENT (#2379). There is nothing on disk to empty, so that line was a false alarm: the +// window answers "Cleared the error log." for `leaf: 'absent'`, and the stderr line reached +// support bundles where it could not be told apart from a leaf that really was refused. +test('a clear with no log on disk stays quiet', async (t) => { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) + const rt = host(home) + t.after(async () => { await rt.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }) + + const stderr = [] + const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) + const result = await rt.diagnostics.clear() + quiet.mock.restore() + + assert.deepEqual(result, { leaf: 'absent', rotated: 'none' }, 'nothing was there, so nothing was kept') + assert.deepEqual(stderr, [], 'a Clear with nothing to empty does not claim it could not truncate') + assert.equal(existsSync(join(home, 'logs')), false, 'the truncate did not create the folder') +}) + +// The other half of the same contract, and #2364 kept whole: a leaf that is there and +// refuses the truncate still names the code on exactly one line, and never the folder (#2379). +test('a clear that cannot truncate the leaf still says so on one line', async (t) => { + if (process.getuid?.() === 0) { + t.skip('root truncates a leaf without its write bit') + return + } + const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + mkdirSync(join(home, 'logs'), { recursive: true }) + const logPath = join(home, 'logs', 'diagnostics.jsonl') + const old = 'OLD LINE\n'.repeat(4) + writeFileSync(logPath, old) + chmodSync(logPath, 0o444) + const rt = host(home) + + const stderr = [] + const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) + const result = await rt.diagnostics.clear() + quiet.mock.restore() + + assert.deepEqual(result, { leaf: 'kept', rotated: 'none' }, 'the leaf says it was left alone') + assert.deepEqual(stderr, ['[ds-bot] diagnostics log not truncated: EACCES'], 'one line, the code and never the folder') + assert.equal(readFileSync(logPath, 'utf8'), old, 'the lines the answer did not claim to clear are still there') +}) diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index e7bb9d5f..c7a6e2c4 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -5126,7 +5126,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js const list = await entries() const client = sanitizeClient(body.client) const clientEntries = sanitizeClientEntries(body.clientErrors) -@@ -232,17 +232,72 @@ export function install(rt) { +@@ -232,17 +232,78 @@ export function install(rt) { } async function logText() { @@ -5165,11 +5165,17 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js + await writeFile(logPath, '', 'utf8') + result.leaf = 'truncated' + } catch (error) { -+ result.leaf = error?.code === 'ENOENT' ? 'absent' : 'kept' -+ // The answer alone is not enough: it reaches the window, and the host log said nothing about -+ // a leaf left full. `append` names this same class of failure on one line, so clear does too; -+ // the line carries the code and never the folder, and the throw stays so the chain ends here. -+ console.error('[ds-bot] diagnostics log not truncated:', error?.code ?? clean(error?.message ?? String(error))) ++ // `ENOENT` is not a refusal: there was no leaf on disk to empty, so there was nothing to ++ // truncate — the same answer the `!toFile` host above gives, and it says so on no line at ++ // all. Every other code is a leaf we wanted to empty and could not: the window hears ++ // `kept`, and the host log names the code on exactly one line (#2364), never the folder. ++ // The throw stays so the chain ends here either way. ++ if (error?.code === 'ENOENT') { ++ result.leaf = 'absent' ++ } else { ++ result.leaf = 'kept' ++ console.error('[ds-bot] diagnostics log not truncated:', error?.code ?? clean(error?.message ?? String(error))) ++ } + throw error + } + // Rotation moves the full log to `.1`, and the truncate alone left every rotated line diff --git a/patches/ds-bot/0007-version-and-tooling.patch b/patches/ds-bot/0007-version-and-tooling.patch index 4b08f792..5789f221 100644 --- a/patches/ds-bot/0007-version-and-tooling.patch +++ b/patches/ds-bot/0007-version-and-tooling.patch @@ -1198,7 +1198,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js const short = issueUrl(base, { what: 'Writer failed\nin the group', diagnostics: '| DS Bot | 0.1.0 |' }) assert.equal(short.inline, true) const params = new URL(short.url).searchParams -@@ -253,3 +253,401 @@ test('a browser error is noted once, repeats are counted, and clear empties the +@@ -253,3 +253,446 @@ test('a browser error is noted once, repeats are counted, and clear empties the clearClientErrors() assert.equal(clientErrors().length, 0) }) @@ -1600,3 +1600,48 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js + assert.deepEqual(await rt.diagnostics.entries(), [], 'a real clear lists nothing afterwards') + assert.equal(await rt.diagnostics.logText(), '', 'a real clear leaves nothing to download') +}) ++ ++// A Clear on a home with no log folder used to print "not truncated" for a write that failed ++// with ENOENT (#2379). There is nothing on disk to empty, so that line was a false alarm: the ++// window answers "Cleared the error log." for `leaf: 'absent'`, and the stderr line reached ++// support bundles where it could not be told apart from a leaf that really was refused. ++test('a clear with no log on disk stays quiet', async (t) => { ++ const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) ++ const rt = host(home) ++ t.after(async () => { await rt.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }) ++ ++ const stderr = [] ++ const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) ++ const result = await rt.diagnostics.clear() ++ quiet.mock.restore() ++ ++ assert.deepEqual(result, { leaf: 'absent', rotated: 'none' }, 'nothing was there, so nothing was kept') ++ assert.deepEqual(stderr, [], 'a Clear with nothing to empty does not claim it could not truncate') ++ assert.equal(existsSync(join(home, 'logs')), false, 'the truncate did not create the folder') ++}) ++ ++// The other half of the same contract, and #2364 kept whole: a leaf that is there and ++// refuses the truncate still names the code on exactly one line, and never the folder (#2379). ++test('a clear that cannot truncate the leaf still says so on one line', async (t) => { ++ if (process.getuid?.() === 0) { ++ t.skip('root truncates a leaf without its write bit') ++ return ++ } ++ const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) ++ t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) ++ mkdirSync(join(home, 'logs'), { recursive: true }) ++ const logPath = join(home, 'logs', 'diagnostics.jsonl') ++ const old = 'OLD LINE\n'.repeat(4) ++ writeFileSync(logPath, old) ++ chmodSync(logPath, 0o444) ++ const rt = host(home) ++ ++ const stderr = [] ++ const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) ++ const result = await rt.diagnostics.clear() ++ quiet.mock.restore() ++ ++ assert.deepEqual(result, { leaf: 'kept', rotated: 'none' }, 'the leaf says it was left alone') ++ assert.deepEqual(stderr, ['[ds-bot] diagnostics log not truncated: EACCES'], 'one line, the code and never the folder') ++ assert.equal(readFileSync(logPath, 'utf8'), old, 'the lines the answer did not claim to clear are still there') ++})