Skip to content

client: a host refusal with an empty message is thrown as an empty error, so the row settles on Loading… - #2414

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-api-empty-error-message
Nov 4, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-api-empty-error-message

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2411
Owner: Dev-2
Written-by: Dev-2
Reviewer: Birfy
Claim-Id: e243cc9423004c24930321eb7fe1816c
Branch: dev/2-api-empty-error-message
Tip: a0386f4
Tree: 377cc239d5a54b30e66bdf85d888248ef68491b1
Base: 67aead2
Diff: 8 files, +344/−11 (hand-written 4 = +165/−3; generated 4 = +179/−8)
Tier: T1 (why: [])

What changed

core/src/client/api.js:38 read a host refusal's message with a nullish fallback: hostText(result.error?.message ?? 'Request failed'). An empty string is not nullish, so ?? kept it, hostText('') returned '', and the client threw new Error('') — every surface that prints error.message drew nothing. On the memory row it was worse: memoryCountText tests if (failure), so an empty reason is falsy, count stays undefined, and the row settles on Loading… for a read that had already failed.

Both places now fall back with || — the thrown error at :38 and the diagnostics copy at :41 — so an empty host message reads Request failed ('Request failed': '请求失败' is already in i18n.js, so no new key and no t() wrapper, which would change the English path). A non-empty host sentence is passed through byte-for-byte, and error.code / error.details still pass through. core/src/host/** is untouched.

Rebased onto 67aead26 (after #2408 and #2405); the generated client.js and the 0008/0012 patches were rebuilt with npm --prefix core run build + node scripts/ds-bot-patches.mjs generate, not hand-edited. #2408's new role="status" note below the row draws the same memoryCountText sentence, so it now announces the refusal instead of Loading…; its truth guard for an empty failure stays in P-538, as agreed, and is not touched here.

Red first

On a clean 67aead26 worktree with only core/test/api-empty-message.test.js added, the file was 5 tests / 1 pass / 4 fail / 0 cancelled (log tmp/p2411-red-67aead26.log, exit 1):

  • bot/refused + message: '' → error.message === '' ("an empty host message must not become an empty error");
  • the memory row's bt-mem-row-copy text was Loading… ("a settled failure never draws the pending text");
  • bot/error + message: '' → the thrown copy was '' and the diagnostics line ended in memory/view: (empty sentence after the separator);
  • bot/internal + message: '' → the sentence was ' Settings → Feedback has the details.' (leading space).

The one passing test is the non-empty host sentence, kept as a regression guard. With the fix the file is 5 / 5 / 0 / 0 on the same base.

Ran

TZ=UTC ARCH_DOCS_BASE=67aead26 npm run verify
node .github/scripts/changelog.mjs check
node .github/scripts/check-empty-catch.mjs --check
node .github/scripts/check-mojibake.mjs --check
node scripts/ds-bot-patches.mjs generate   (twice: the second run leaves the tree clean)
node scripts/ds-bot-patches.mjs canonical

4831 = 2844+570+132+140+411+671+37+26, 0 fail / 0 cancelled; lint 0 errors (core 76 warnings, plugins 36/9/2/16); arch-docs: OK (8 changed file(s)); 10 patches apply cleanly and core/ equals ds-bot + patches; build-client --check exit 0 (client.js is the build of src/client); changelog 30 fragments well formed; empty-catch exit 0; mojibake 0 U+FFFD in 1464 text files; generate a no-op on the second run and canonical ok.

Not run

The smoke test (this touches neither index.js nor src/host/), the pack check / typecheck / actionlint of CI's package job, and CI itself.

Docs: docs/subsystems/core-client.md — the error-toast bullet now states the transport fallback and names core/test/api-empty-message.test.js.

Rebased onto the new main on the Lead's instruction; kept as a draft until the Lead sends it to Test.

{
"pr": 2414,
"claim": "Dev-2",
"issue": 2411,
"summary": "api.js read a host refusal's message with a nullish fallback, so an empty message survived as new Error('') and every surface printing error.message drew nothing; the memory row settled on Loading… for a read that had already failed. Both the thrown error and the diagnostics copy now fall back with ||, so an empty host message reads Request failed, a sentence already in i18n; a non-empty sentence, the code and the details are unchanged. Five tests cover both branches, the row, the leading space and the non-empty pass-through. Rebased onto 67aead2 with the bundle and patches regenerated.",
"files": [
"changelog.d/2411-client-empty-error-message.fixed.md",
"core/client.js",
"core/src/client/api.js",
"core/test/api-empty-message.test.js",
"docs/subsystems/core-client.md",
"patches/ds-bot/0008-client-bundle.patch",
"patches/ds-bot/0012-room-membership.patch",
"patches/ds-bot/files/test/api-empty-message.test.js"
],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=67aead26 npm run verify",
"result": "green",
"at": "a0386f4d773dca6e914530c67d5cf89a3c8d1463"
},
"next": "draft on purpose; rebased onto 67aead2, waiting for the Lead to send it to Test"
}

@HugoXK
HugoXK marked this pull request as draft November 4, 2026 15:22
`??` keeps an empty string, so a refusal whose `message` was '' was thrown
as `new Error('')`: every surface that prints `error.message` drew nothing,
and the memory row — whose sentence tests the failure for truth — settled on
"Loading…" for a read that had already failed.

Answer the empty message with `Request failed` on both the thrown error and
the noted copy, the sentence already in i18n for zh. A non-empty host
sentence is unchanged.

Fixes ThinkFlowLab#2411
@HugoXK

HugoXK commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor Author

@Birfy review request — #2414 at tip a0386f4d773dca6e914530c67d5cf89a3c8d1463 (T1, client api.js, Fixes #2411).

As of my read (GitHub Date 16:13Z): draft false (flipped after the advisory), mcm true, base 67aead26239b59ca9197104b6341f74568e5a660 = live main, 8 files +344/−11, mergeable true. Check runs at this tip: 1 success + 16 queued, 0 failure — the runners are still picking the batch up, so tests-ok has not reported yet.

Independent advisory (FlyBot Test) at this exact tip says PASS: red first on clean 67aead26 with only the new test = 5 = 1 pass + 4 fail, then 5/5 at the tip; a non-empty host sentence, code and details pass through unchanged; zh reads 请求失败 (no new i18n key, i18n.js zero diff); core/src/host/** zero diff; generate twice a no-op, patches check 10/10 and core/ = ds-bot/ + patches, files/ mirrors byte-equal; TZ=UTC ARCH_DOCS_BASE=67aead26 npm run verify 4831 = 2844+570+132+140+411+671+37+26, 0 fail / 0 cancelled.

The change itself is two characters' worth of behaviour: core/src/client/api.js:38 and :41 read the host message with || instead of ??, so an empty host message becomes Request failed (an existing i18n string) instead of new Error(''); every surface that prints error.message stopped drawing a blank, and the memory row no longer settles on Loading… for a read that failed. Generated core/client.js, patches/ds-bot/0008-client-bundle.patch and patches/ds-bot/0012-room-membership.patch are a rebuild, not hand edits.

@github-actions github-actions Bot added the risk:T1 Risk tier T1 (client, tests, generated files), set by the controller from the diff label Nov 4, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 4, 2026
Merged via the queue into ThinkFlowLab:main with commit a091ddc Nov 4, 2026
18 checks passed
@Birfy
Birfy deleted the dev/2-api-empty-error-message branch November 4, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:T1 Risk tier T1 (client, tests, generated files), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

client: a host refusal with an empty message is thrown as an empty error, so the row settles on Loading…

2 participants