fix: coded errors survive the shard boundary, plus two codegen losses - #332
Merged
Conversation
`defineTable({ status })` — a shorthand property whose value is a validator
held in a const — was dropped from the table shape entirely. `parseObjectShape`
skipped anything that was not a `PropertyAssignment`, and a shorthand property
is its own initializer, so the column vanished from `Doc_*` with no error
anywhere.
The failure is silent and arrives late: an index over the missing column
surfaces only as a confusing `index_references_unknown_field` advisory pointing
at a column the author can plainly see in the schema, and a column with no index
produces no diagnostic at all — just a runtime insert that writes a field the
generated types say does not exist. `object-shorthand` autofixes
`status: status` into this form, so the loss can arrive from a lint run on a
schema that was previously correct.
Every caller of the parser was affected: table shapes, `.input()` args, http
routes and mutators all read their shape through it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYvgKochPxnrCCtKB1tXGf
A handler annotated with a type it exports from its own module emitted that
bare name into `_generated/api.ts`, which never imports the handler's module —
so the generated file did not compile (TS2304) while `lunora codegen` exited 0.
`symbolDeclaredUnreachable` treated an exported declaration as reachable, on the
assumption that an exported name can be imported. Nothing emits that import: the
emitter only rewrites qualifiers the type checker itself rendered as
`import("…")`, and the checker prints an exported local type by bare name
precisely because it is nameable from the handler.
A non-exported declaration was already expanded structurally. Exported ones now
take the same path, so the emitted type is identical in shape and resolves from
anywhere. The existing test asserted the old behaviour on the stated grounds
that "the name is valid"; it encoded the defect, and is rewritten.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYvgKochPxnrCCtKB1tXGf
Every `LunoraError` a mutation handler threw reached the client as a generic 500 `INTERNAL` / "Internal error". A `NOT_FOUND`, a `CONFLICT` on a unique index, an `UNAUTHENTICATED` guard — all indistinguishable, all logged as internal faults, and all unbranchable by the client. Queries were unaffected because they never enter a transaction, which made the behaviour look arbitrary. Two boundaries in the mutation path lose the error: 1. `blockConcurrencyWhile` (the single-writer gate) treats a rejecting closure as unrecoverable and ABORTS the Durable Object. That is the right default for the initialization work the API was designed for, and the wrong one for a gate wrapping every mutation: an ordinary application error tore down the shard, discarding its in-memory state and every hibernating WebSocket subscription on it, for every other client connected to that shard. 2. `storage.transaction` rolls back correctly but propagates a flattened copy — a plain `Error` carrying `name: message` and none of the original's own properties. `isLunoraError` is structural (`type`/`code`/`status`), so the copy fails that check and is redacted. Both now settle the closure's outcome inside the boundary and re-raise the original instance outside it. Semantics are otherwise unchanged: the gate is still held for the whole closure, and the transaction closure still throws into the platform so the rollback happens exactly as before. A failure raised by the platform itself, with no handler error to restore, is surfaced untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYvgKochPxnrCCtKB1tXGf
✅ Deploy Preview for lunorash ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
WalkthroughThe PR updates codegen reachability and shorthand validator parsing. It also changes Cloudflare concurrency and transaction handling so application errors are preserved across platform boundaries. ChangesGenerated API type handling
Cloudflare application error propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant runSerialized
participant blockConcurrencyWhile
participant transaction
Application->>runSerialized: Invoke application closure
runSerialized->>blockConcurrencyWhile: Execute boxed closure
blockConcurrencyWhile-->>runSerialized: Return boxed result or error
runSerialized-->>Application: Return result or rethrow original error
Application->>transaction: Invoke transactional closure
transaction-->>Application: Rethrow closure error after rollback
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thank you for following the naming conventions! 🙏 |
Contributor
|
Thank you for confirming the Contributor License Agreement! 🙏 |
Merging this PR will degrade performance by 23.05%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
This was referenced Aug 2, 2026
This was referenced Aug 3, 2026
Merged
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
1 participant
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.
Three defects, all found by running apps against the framework rather than by reading it. Each is silent: the code compiles, the command exits 0, and the damage shows up at runtime or not at all.
1. A thrown
LunoraErrornever reached the client (@lunora/platform-cloudflare)Every coded error a mutation handler threw arrived as a generic 500
INTERNAL/ "Internal error".NOT_FOUND,CONFLICTon a unique index, anUNAUTHENTICATEDguard — all identical on the wire, all logged as internal faults, none branchable by the client. Queries were unaffected (they never enter a transaction), which is what made it look arbitrary.Two boundaries in the mutation path lose it:
blockConcurrencyWhile— the single-writer gate. workerd treats a rejecting closure as unrecoverable and aborts the Durable Object, discarding its in-memory state and every hibernating WebSocket subscription attached to it. So one user's "not your turn" error tore the shard down for everyone else connected to it. That default is right for the initialization work the API was designed for; it is wrong for a gate wrapping every mutation, where the transaction inside has already rolled the writes back and the object's state is well defined.storage.transaction— rolls back correctly, but propagates a flattened copy: a plainErrorcarryingname: messageand none of the original's own properties.isLunoraErroris structural (type/code/status), so the copy fails the check and gets redacted.Both now settle the closure's outcome inside the boundary and re-raise the original instance outside it. The gate is still held for the whole closure; the transaction closure still throws into the platform, so rollback is unchanged. A failure raised by the platform itself — with no handler error to restore — is surfaced untouched.
Before / after, same handler, same request:
2. Shorthand table columns were dropped (
@lunora/codegen)defineTable({ status })lost the column entirely —parseObjectShapeskipped anything that was not aPropertyAssignment, and a shorthand property is its own initializer.It fails quietly and late. With an index over the column you get an
index_references_unknown_fieldadvisory naming a column that is plainly there in the schema; without one you get no diagnostic at all, just a runtime insert writing a field the generated types deny.object-shorthandautofixesstatus: statusinto this form, so a lint run can introduce it into a schema that was correct yesterday. Table shapes,.input()args, http routes and mutators all read their shape through this parser.3. Exported handler return types emitted unresolvable names (
@lunora/codegen)A handler annotated with a type exported from its own module emitted that bare name into
_generated/api.ts— which never imports the handler's module. The generated file failed to compile (TS2304) whilelunora codegenexited 0.symbolDeclaredUnreachableassumed an exported name is importable. Nothing emits that import: the emitter only rewrites qualifiers the checker itself rendered asimport("…"), and the checker prints an exported local type by bare name precisely because it is nameable from the handler. Non-exported declarations were already expanded structurally; exported ones now take the same path.The existing test asserted the old behaviour on the stated grounds that "the name is valid" — it encoded the defect, so it is rewritten.
Verification
cloudflare-host.transaction.test.ts(7 cases) covers error identity, the abort, rollback ordering, platform-raised failures, non-Errorthrows, and the composed gate+transaction path. Confirmed failing without the fix.platform-cloudflare31,do519,shard-engine967,runtime833,codegen1045.api:checkgreen (44 snapshots) — no public surface change.BAD_REQUEST/CONFLICTand carrying its HTTP status.🤖 Generated with Claude Code
https://claude.ai/code/session_01CYvgKochPxnrCCtKB1tXGf
Summary by CodeRabbit