Observation-class finding, spotted while reading the boundary for objectui#5902. Not touched by that PR — its dispatch scoped packages/data-objectstack out (the boundary work from objectui#5901 is done and merged), so this is recorded rather than edited.
What
packages/data-objectstack/src/index.ts, the tail of normaliseClientError:
if (e.code !== 'CONCURRENT_UPDATE' && e.httpStatus !== 409) return error;
if (e.code !== 'CONCURRENT_UPDATE') return error;
return new ConcurrentUpdateError({ ... });The second line's condition is strictly weaker than the first's, so the first line can never be the one that decides an outcome:
code |
httpStatus |
line 1 |
line 2 |
outcome |
≠ CONCURRENT_UPDATE |
≠ 409 |
returns |
would also return |
passthrough |
≠ CONCURRENT_UPDATE |
409 |
falls through |
returns |
passthrough |
CONCURRENT_UPDATE |
any |
falls through |
falls through |
typed error |
Delete line 1 and every input keeps its current answer. The httpStatus !== 409 half in particular reads as if a bare 409 could still be re-wrapped; it cannot.
Why it is worth a card rather than a silent cleanup
It is dead code, not a behaviour bug — the effective rule is "the wire code is the discriminator", which is a reasonable rule and the one the function's own doc comment describes. What makes it worth recording is that the line advertises a second acceptance path that does not exist, on a function whose whole job is deciding which errors get re-wrapped. A reader adding a shape here has to re-derive the truth table to find out that half of the first guard is inert.
There is a related asymmetry worth deciding at the same time: the exported isConcurrentUpdateError in the same file accepts name === 'ConcurrentUpdateError' as well as the code, while normaliseClientError accepts only the code. Consumers duck-typing through the exported predicate and consumers relying on the re-wrap therefore disagree about one shape.
Severity
Low, and deliberately filed flat rather than pre-graded — no user-visible symptom, no test moves either way. Triage decides whether it is worth a patch at all or whether the right move is a one-line comment naming the code as the sole discriminator.
Where
packages/data-objectstack/src/index.ts — normaliseClientError, the two guards immediately before return new ConcurrentUpdateError(...)packages/data-objectstack/src/index.ts — isConcurrentUpdateError, for the asymmetry above
Found from: objectui#5902 (drag-write surfaces reading the producer-marked userMessage).
Observation-class finding, spotted while reading the boundary for objectui#5902. Not touched by that PR — its dispatch scoped
packages/data-objectstackout (the boundary work from objectui#5901 is done and merged), so this is recorded rather than edited.What
packages/data-objectstack/src/index.ts, the tail ofnormaliseClientError:The second line's condition is strictly weaker than the first's, so the first line can never be the one that decides an outcome:
codehttpStatusCONCURRENT_UPDATECONCURRENT_UPDATECONCURRENT_UPDATEDelete line 1 and every input keeps its current answer. The
httpStatus !== 409half in particular reads as if a bare 409 could still be re-wrapped; it cannot.Why it is worth a card rather than a silent cleanup
It is dead code, not a behaviour bug — the effective rule is "the wire
codeis the discriminator", which is a reasonable rule and the one the function's own doc comment describes. What makes it worth recording is that the line advertises a second acceptance path that does not exist, on a function whose whole job is deciding which errors get re-wrapped. A reader adding a shape here has to re-derive the truth table to find out that half of the first guard is inert.There is a related asymmetry worth deciding at the same time: the exported
isConcurrentUpdateErrorin the same file acceptsname === 'ConcurrentUpdateError'as well as the code, whilenormaliseClientErroraccepts only the code. Consumers duck-typing through the exported predicate and consumers relying on the re-wrap therefore disagree about one shape.Severity
Low, and deliberately filed flat rather than pre-graded — no user-visible symptom, no test moves either way. Triage decides whether it is worth a patch at all or whether the right move is a one-line comment naming the code as the sole discriminator.
Where
packages/data-objectstack/src/index.ts—normaliseClientError, the two guards immediately beforereturn new ConcurrentUpdateError(...)packages/data-objectstack/src/index.ts—isConcurrentUpdateError, for the asymmetry aboveFound from: objectui#5902 (drag-write surfaces reading the producer-marked
userMessage).