Conversation
CachingBodyFilter caught ServletException/IOException from the chain, logged it and returned. Nothing then set a status, so the response went out at Tomcat's default 200 with an empty body — a request the server dropped, reported to the caller as a success. DispatcherServlet wraps everything a handler throws, Errors included, into "Handler dispatch failed", so this masked every unhandled failure on every non-streaming endpoint. Seen in the e2e run as POST /timeseries/data answering 200 instead of 204 while the api was throwing OutOfMemoryError. copyBodyToResponse() moves into a finally so a body written before the failure still reaches the caller instead of being discarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: jgjesdal <jostein@intellistream.ai>
CachingBodyFilter caught IOException and ServletException from the rest of the chain and only logged them. DispatcherServlet wraps anything no handler answered in a ServletException, an Error included, so every such failure left the filter with no status set and went out as 200 with an empty body. The e2e's Rust `test_datapoints` hit it: 52 concurrent 100 000-point JSON inserts against the CI api's 384 MiB heap (one such body costs about 20 MiB parsed), and the SDK got 200 where the contract says 204. Every SDK treats a 2xx as stored. The filter now lets the failure propagate on both of its paths, so the container answers it with a 500 the SDKs retry. The body is copied to the response only on success; copying a half-written one would commit the response before the 500 could be sent. DatapointInsertHttpTest drives POST /timeseries/data over real HTTP through the production filter chain with the service throwing OutOfMemoryError: 500 now, and `expected: 500 but was: 200` against the old filter. CachingBodyFilterTest pins propagation on the wrapped and streaming paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: olav <olav@intellistream.ai>
…onverter builder `WebMvcConfigurer.extendMessageConverters(List)` is deprecated for removal in Spring Framework 7 ([removal] warning in the e2e build). The replacement is `configureMessageConverters(HttpMessageConverters.ServerBuilder)`. The swap goes through `configureMessageConvertersList` rather than `withJsonConverter`: Spring Boot names its own JSON converter on the same builder, so whichever configurer ran last would win. A list configurer runs inside `build()` after all of them, sees the converter Boot settled on, and replaces it in place as before. StrictRequestBodyConfigTest now also drives the real builder with the JSON converter named after this config has run. The booted-application check that an unknown field is still a 400 naming it is in DatapointInsertHttpTest, added in the previous commit; with this hook disabled that request reaches the controller instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: olav <olav@intellistream.ai>
…rs-never-answer-200 Both branches fixed CachingBodyFilter swallowing failures. The conflicts are resolved to that branch's CachingBodyFilter and CachingBodyFilterTest as they stand. The one difference was where the buffered body is copied: in a finally there, only on success here, on the worry that a half-written body copied after a failure would commit a 200 before the container could send its 500. It does not happen. DispatcherServlet resets an uncommitted response's buffer before handling any exception, so the buffer is empty by the time a failure reaches the filter and the copy is a no-op. Checked over real HTTP: a /timeseries/data/list response that throws OutOfMemoryError after writing about 100 KB of JSON is a 500 with either filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: olav <olav@intellistream.ai>
…er-answer-200 Fix/api errors never answer 200
olavgg
approved these changes
Sep 15, 2026
Contributor
Author
|
missing signatures |
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.
What
CachingBodyFiltercaughtServletException/IOExceptionfrom the filter chain, logged it and returned. Nothing then set a status, so the response went out at Tomcat's default 200 with an empty body — a request the server dropped, reported to the caller as a success.DispatcherServletwraps everything a handler throws —Errors included — intoServletException: Handler dispatch failed, so this masked every unhandled failure on every non-streaming endpoint, not just the odd I/O fault. The streaming branch of the same method had the same swallow, so file download/upload and graph import/export were masking failures too.How it showed up
In the E2E workflow,
POST /timeseries/dataanswered200where the Rust SDK asserts204, with an empty body. The api logs for the same window:86 OOMs, 43 requests silently turned into a 200.
Changes
copyBodyToResponse()moves into afinally. It was previously skipped on the failure path, discarding a body written before the throw. Copying an empty buffer is a no-op, so the container is still free to write its own error page over an uncommitted response.Testing
New
CachingBodyFilterTest— 5 tests, including the exact shape seen in CI (ServletExceptionwrapping anOutOfMemoryError), asserting it propagates and the response stays uncommitted.Full
:datahub-api:testgreen: 789 tests, 0 failures. Nothing depended on the old behaviour.Docs
No docs change. This restores the documented contract (204/404/422) rather than altering it.
Note
This does not on its own make E2E green — the OOM behind it is fixed separately in #105.
🤖 Generated with Claude Code