Improve TLS-rejection diagnosis and visibility - #261
Merged
taylorcox75 merged 1 commit intoSep 21, 2026
Merged
Conversation
A rejected self-signed certificate was indistinguishable from a dead
server, so nobody could tell what was wrong. This does not fix the
reporter's underlying connection failure (still awaiting repro details
on the issue) — it ships the diagnosis/visibility half:
- utils/error.ts: isTlsRejection(error) recognizes iOS rejecting a
server's TLS certificate from the free-text description RN's XHR
bridge exposes on error.request.response, matched across all six
locales since that text is localized to the device language.
- services/api/client.ts: the network-error branch throws a distinct
"Certificate rejected..." message (with a clogWarn('TLS', ...)) when
isTlsRejection matches, instead of the generic connection-timeout
message, without touching the existing substring-matched strings.
- modules/insecure-cert-allowlist: exports
isInsecureCertAllowlistAvailable() so a server wanting the allowlist
flag on a binary that predates the native module (an OTA JS update
shipped without it) is surfaced instead of silently no-op-ing —
services/server-manager.ts warns via clogWarn('CERT', ...), and the
server add/edit screens show a hint under the toggle.
- components/SuperDebugPanel.tsx: both REACH probes switch from
fetch() to a raw XMLHttpRequest so the certificate-specific guidance
(previously unreachable, since whatwg-fetch collapses every network
failure into a bare "Network request failed") can actually fire.
- AGENTS.md: File Index updates plus a new §10 Gotcha entry about an
OTA-shipped feature silently lacking its native half.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related to #256. The reporter's underlying connection failure is not fixed here — that's still waiting on repro details the owner requested on the issue (does the toggle still show ON, is the failure instant or ~10s, a connectivity-log screenshot). What ships now is the diagnosis/visibility half described in the plan's "What you CAN ship now" section, so a rejected certificate stops being indistinguishable from a dead server:
utils/error.ts— newisTlsRejection(error)recognizes iOS rejecting a server's TLS certificate from the free-text description React Native's XHR bridge puts onerror.request.response(the native NSError'slocalizedDescription— RN forwards only that string, not the NSURLErrorDomain code). Matches language-neutral SSL/TLS terms plus "certificate" translated into all six locales this app ships, since that description text is localized to the device's language.services/api/client.ts— the network-error branch (ECONNABORTED/ERR_NETWORK) now checksisTlsRejectionfirst and throws a new, distinct message ("Certificate rejected. Enable...") with aclogWarn('TLS', ...), instead of folding it into the existing'Connection timeout...'string that callers substring-match. The new message is deliberately kept out ofRECONNECTABLE_MESSAGES— a rejected cert isn't something a re-login fixes, so it shouldn't trigger a reconnect loop.modules/insecure-cert-allowlist/index.ts— newisInsecureCertAllowlistAvailable(). OTA JS updates ship independently of the native binary, so a device can get this module's JS wrapper without its native half (a binary predating #... whichever PR added it). Previously the toggle just silently no-op'd with zero signal.services/server-manager.ts'ssyncInsecureCertAllowlistnow warns viaclogWarn('CERT', ...)when a server wants the flag but it's unavailable, andapp/server/add.tsx/app/server/[id].tsxshow a hint under the toggle ("Requires the latest App Store version") in that case.components/SuperDebugPanel.tsx— both REACH probes (Feature 1 "Ping Host" and Step 1 of the full diagnostic run) switch fromfetch()to a rawXMLHttpRequest. The certificate-specific guidance text existed but was unreachable: RN'swhatwg-fetchpolyfill collapses every network-level failure, including a rejected cert, into a bareTypeError('Network request failed'), so the generic branch always won. The raw XHR preserves the response-body detailisTlsRejectionneeds, same as the app's real HTTP client already relies on.AGENTS.md— File Index updates for the above, plus a new §10 Gotcha: a native-module-backed feature can be delivered via OTA to a binary that lacks the native module, and the JS wrapper no-ops silently rather than erroring — check availability explicitly.Scope note: per the plan, this branch does not touch
app/(tabs)/(torrents)/index.tsx,context/TorrentContext.tsx, orcontext/TransferContext.tsx(owned by other tasks in this milestone) beyond the two files (services/api/client.ts,services/server-manager.ts) it was explicitly blocked-then-unblocked on Task A (#254, already merged) to edit.Test plan
npx tsc --noEmit→ exit 0npm test→ 79 suites / 1160 tests, all passing (includes newisTlsRejectionunit tests intests/utils/error.test.tsand new network-error-branch tests intests/services/client.test.ts, covering an English description, a non-English description, and a plain ERR_NETWORK that must not be misclassified)npm run lint→ 0 errors, 37 warnings (documented baseline)npm run format→ applied🤖 Generated with Claude Code