Basekick-Labs / Basekick-Labs/arc
arcx: the Arrow IPC stream writer recovers from a panic without the #729 poison marker and truncation trailer
- Dominant language
- Go
- Stars
- 677
- Forks
- 53
- Avg merge
- 9h 14m
- Merged PRs (30d)
- 164
Description
Follow-up from the #717 review of PR #845, arcx-only (builds with the `arcx_engine` tag; no release ships it).
PR #845 wraps the three body-stream writers in `internal/api/arcx_hook.go` in `safeStream`, so a panic inside one fails the request instead of crashing the process. For the msgpack and JSON writers that is the same disposition the stock writers have. The Arrow IPC writer (`tryArcxRouterArrow`) is not quite the same: the stock Arrow IPC writer in `query_arrow.go` additionally poisons the stream (`poisonArrowStream`) and sets the `arrowStreamTruncatedTrailer` on a panic (#729), so an Arrow reader cannot mistake a stream cut at a batch boundary for a complete one. The arcx site has neither, so after #845 a panic there leaves the client with a stream that ends cleanly at the last written batch and no end-of-stream marker or trailer.
Before #845 the same panic took the process down, so this is strictly an improvement, but the arcx Arrow path should adopt the #729 poison + trailer before the arcx engine becomes the default engine. The site is also still uncapped (see the #702 note above the hook call in `query_arrow.go`).
Fix shape: route the arcx Arrow IPC site through the same `setBodyStreamWithTrailers` + `poisonArrowStream` path the stock writer uses, or factor that pair into a helper both call.
Contributor guide
Research direction
Read internal/api/arcx_hook.go and compare tryArcxRouterArrow with the stock Arrow path in query_arrow.go, including the #702 note. Trace how setBodyStreamWithTrailers and poisonArrowStream are used, then verify the arcx path marks a panic-truncated stream with the poison marker and arrowStreamTruncatedTrailer without crashing the process.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100