Rest client catch up - #411
Merged
Merged
Conversation
Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
Guard lazy query parsing and add a regression test for malformed redirects. Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
- Support fixed-count and AllAvailable batches from a requested index. - Preserve per-message metadata and report gaps in both REST clients. - Continue polling after malformed batches. - Fix subscription cleanup to allow unsubscribe and resubscribe. - Add tests and document batch behavior. Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
drslebedev
had a problem deploying
to
configure coverage
September 11, 2026 10:37 — with
GitHub Actions
Failure
|
RalphSteinhagen
approved these changes
Sep 11, 2026
RalphSteinhagen
left a comment
Member
There was a problem hiding this comment.
Reviewed the server, both clients, tests and docs; built locally with clang20 Debug and ran the long-polling/batch tests, all green. The clang20 CI failure is the unrelated OAuth Worker test (Keycloak login simulation).
Batch parameters are bounded, invalid requests are rejected before any side effect, and the multipart format is injection-safe. 👍
The following points below are non-blocking and fine as follow-ups:
- Caching:
AllAvailableresponses are not immutable for a given URL, unlike indexed single-message responses. Addcache-control: no-storeto batch responses so a caching proxy can never serve a stale, shorter batch. - Range guard in
respondToLongPollBatch:firstIndex - entry.firstIndexis unsigned and the deque access is unchecked. Current dispatch guarantees the range, buthandleNotificationalready tests readiness with>=, so clamping to[entry.firstIndex, entry.nextIndex())is cheap insurance against an out-of-bounds read. - Error notifications in batches arrive as data. A per-part error header would preserve
Message::error; the single-message path has the same limitation. - Recovery after an undecodable batch resumes with
Nextand drops what is still buffered. Retrying once from last+1 before falling back would lose less. - Memory: the batch body is built as a string and then copied into an
IoBuffer, up to 100 messages twice, with no byte cap. Encoding into theIoBufferdirectly and cappingAllAvailableby bytes bounds this.
If you need to touch the code anyway, I'd suggest to at least tackle the first point because this has an impact on our local testing and use-case.
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
Adds indexed batch catch-up for REST subscriptions, allowing clients to retrieve buffered messages with fewer requests after falling behind.
Related to #405.
Changes
Add LongPollingBatch support to the server and native/Emscripten clients:
Start from the oldest retained message when the requested batch index is too old. Fixed-count batches still return exactly the requested count.
Preserve each message’s index, topic, service name, and payload in multipart responses.
Report skipped samples alongside valid data and continue polling after malformed batches.
Fix subscription cleanup so clients can unsubscribe and subscribe again.
Catch native GET/SET and redirect query-parsing errors instead of allowing exceptions to escape.
Suppress third-party libsodium compiler warnings for GCC/Clang.
Document batch behavior and the response format.
Requests without LongPollingBatch still return one message. Proxy caching policy is unchanged; general automatic recovery remains separate work.
Test coverage
Adds or extends tests for multipart encoding/decoding, batch ranges and buffer retention, invalid requests, subscription lifecycle, and query-parsing failures. Integration coverage includes both native
polling modes and Emscripten batch delivery, gap warnings, and malformed-batch recovery.