Repository navigation
test(oxia): record what the connect failure is, and what it is not - #1804
Merged
Merged
Conversation
added 2 commits
October 16, 2026 11:39
Three runs on 2026-10-16 failed in this file at connect, each a different case. The record of what
was measured now sits beside the server that binds, because the next red will be read against it
and the first reading of this one was wrong.
WHAT IT IS NOT: a refusal. The category was `not-connected`, which
`src/lib/db/providers/keyvalue/oxia/errors.ts:294-304` reaches only when the details hold "No
connection established" and carry no errno. A refusal reads `refused` there instead - confirmed by
shutting a server down on purpose and watching the category change, which is how the first reading
("it failed in milliseconds, so the socket was not accepting yet, so it was refused") was found to
be wrong. Failing in milliseconds was the right observation; "refused" was a step that had not been
measured.
WHAT IT IS, as far as it was measured: gRPC code 14, "No connection established", no errno, in
3,14 / 17,38 / 8,70 ms against a 3.000 ms deadline.
WHAT MAKES THIS FILE THE ONE: nine files here bind a gRPC server, none waits for the server to be
accepting, eight bind `127.0.0.1:0` and dial the same family, and this one binds `[::]` while
`optionsFor` dials `127.0.0.1`. The `[::]` bind stays - the cases that read `localhost`, `[::1]` and
`[0:0::1]` need one server all three reach.
WHY THERE IS NO FIX HERE: the race did not reproduce in 23 attempts (ten idle, ten under two
concurrent coverage suites, three full suites at --jobs=4, which is what CI runs), and a dial of our
own would not catch it either - 600 of 600 such dials were accepted, 300 idle and 300 at load 37. A
one-shot dial was written and taken back out for exactly that reason: it would have tested a refusal,
and the failure is not one.
Comment only. No behaviour changes.
The record added in the commit before this one said the category is reached "when the details hold
'No connection established' and carry NO errno". Scoped to `errors.ts:294-304` that was nearly
right, and read any wider it is wrong in two ways. Both were measured in the source:
- that path tests THREE strings, not "an errno": `ECONNREFUSED` reads `refused`, `ENOTFOUND` and
`EAI_AGAIN` read `dns`, and everything else falls through. A details carrying `ETIMEDOUT` would
therefore read `not-connected` as well;
- and the category has a second route that is not that path at all. An error raised OUTSIDE a
gRPC status, carrying a Node code, is classified at `errors.ts:381` through `SYSTEM_ERRORS`,
where `EHOSTUNREACH`, `ENETUNREACH` and `ETIMEDOUT` map to `not-connected` - WITH an errno.
So `not-connected` does not mean "no errno" in that module; it means that on the path this file's
failure took. The comment now says which path, and names the other one.
A correction to a comment, in a commit of its own rather than an amend: the branch is already open
as #1804, and rewriting a pushed commit would leave the only way forward a force push.
Comment only. No behaviour changes.
cevheri
approved these changes
Oct 16, 2026
|
yusuf-gundogdu
added a commit
that referenced
this pull request
Oct 16, 2026
… kept main had moved six PRs ahead (#1794, #1799, #1800, #1792 Modbus, #1804, #1801) and GitHub called the branch unmergeable. 73 files conflicted in 131 hunks, and the resolution rule was one sentence: WHERE BOTH SIDES ADDED AN ENTRY, BOTH ENTRIES STAY, main's first because Modbus landed first. NUMBERS DERIVED FROM THE MERGED TREE, not carried from either side: EXTERNAL 39, SHIPPED 40, WIRE_COMPATIBLE 27 (unchanged by either engine), connectable 66. The four denominators stay apart - "thirty-nine engines" is EXTERNAL, "forty type-ids" and "all forty ids" are SHIPPED, "sixty-six named engines" is EXTERNAL plus the wire-compatible relatives. WHAT THE LINE-BY-LINE MERGE GOT WRONG, each caught by a gate rather than by reading: - seven src and four test files lost a closing brace where two entries' tails were identical, so the entry was rebuilt from main's file with this engine's record moved in whole; - `migration-generator.ts` kept the record but lost BOTH plain-list entries; - `dialect-editors.test.ts`, `object-source-declarations.test.ts`, `mcp-documentation.test.ts`, `connection-fingerprint.test.ts` and `BRAND_MESSAGING.md` each carried a duplicated claim, one copy from each side, with different numbers in three of them; - the golden dialect fixture cannot be merged line by line at all: it was regenerated with the command its own docblock gives, and the gate compares it byte for byte; - the operator CSVs were regenerated with `make -C operator bundle` after the source manifest was corrected, not edited by hand. AND THREE DEFECTS OF MY OWN RESOLUTION, found by scanning for them rather than by trusting it: a double comma in README.md, a space before a comma in both chart READMEs, and a stale "and" left in the operator CSV's engine list. The scans are in the record: `,,`, ` ,`, duplicated adjacent lines, and engine-name counts per fleet list. NINE TRANSLATIONS carry both engines in every fleet list and the numeral in each file's own form: a word in en/zh/es/pt/ru/hi/ur and a figure in ja/ko. The Urdu pair had to be matched by code point, because the word I typed was not the word the file holds. THE RECENCY CHAIN MOVED, which a gate holds at both ends: Sparkplug B is now the newest, Modbus came before it, and `not.toContain("Modbus is the newest")` is what keeps the previous head from quietly claiming a title it no longer holds. MEASURED, twice, lane taken, chart dependency built: 1550 files, 57560 tests, 57555 pass, 5 skip, 0 fail, 0 not run, exit 0. format, lint, typecheck, knip, readme:check, security:check, distribution:check, channels:showcase:check and chart:check all exit 0. ONE FINDING THAT IS NOT MINE: main dropped "Apache Spark" from the engine list in docs/AGENT_GUIDE.md (the base has it, origin/main does not). The name is restored here because the line had to be edited for this engine anyway and shipping a list that silently lost an engine is worse; it is reported rather than hidden.
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
2 participants
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.
tests/unit/db/oxia/grpc-client.test.tsfailed three times in one day, each time a different case, and each one in milliseconds. This adds a comment recording what was measured about it, and nothing else: the diff is comment lines only.Why a record rather than a fix
A fix was written and taken back out. The first reading was that a failure in milliseconds, against a 3000 ms deadline, meant a socket that was not accepting yet, and therefore a refusal. A probe was written to catch that. The canary for it compared the two side by side and refuted the reading:
keyvalue/oxia/errors.tsseparates them: a details string holdingECONNREFUSEDreadsrefused, so the CI failure was not a refusal, and the probe would have tested the wrong fact. It was reverted.What is recorded
What it is not. Not a refusal. The category is
not-connected, which that path reaches only when the details hold "No connection established" and carry no errno. Confirmed by shutting a server down on purpose and watching the category change.What it is, as far as it was measured. gRPC code 14, "No connection established", no errno, at 3.14, 17.38 and 8.70 ms against a 3000 ms deadline.
What makes this file different. Nine files in this repository bind a gRPC server. None of them waits for the server to be accepting. Eight bind
127.0.0.1:0and dial the same family. This one binds[::]and dials127.0.0.1, and it is the only one that has been seen to fail at connect. The[::]bind stays: the cases that readlocalhost,[::1]and[0:0::1]need one server all three reach.What is not known. Why. The race did not reproduce in 23 attempts: ten idle, ten under two concurrent coverage suites, and three full suites at
--jobs=4, which is what CI runs. A dial to the bound port was accepted 600 times out of 600 across those conditions, so a probe of our own is not a way to catch it either.What this changes
Nothing runs differently. The next red here should be read against this, and the reading that was already tried and refuted should not be tried again.