Copilot commented on code in PR #3184:
URL: https://github.com/apache/tika/pull/3184#discussion_r4026316310
##########
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ServerProtocolIO.java:
##########
@@ -128,7 +129,21 @@ public void writeFinished(PipesResult pipesResult) throws
IOException {
} catch (IOException e) {
lastRespSerNanos = System.nanoTime() - serStart;
if (!bos.overflowed()) {
- throw e;
+ if (!(e instanceof JsonProcessingException)) {
+ throw e;
+ }
+ // the result itself cannot be encoded (the buffer is in
memory, so this is
+ // not the pipe): report it for this document rather than let
the parent
+ // count a worker crash and lose the document
+ LOG.warn("result could not be serialized; returning a
status-only result", e);
+ PipesResult.RESULT_STATUS status =
alreadyEmitted(pipesResult.status()) ?
+ pipesResult.status() :
PipesResult.RESULT_STATUS.PARSE_EXCEPTION_NO_EMIT;
Review Comment:
Because `writeFinished` is also used for `FETCH_EXCEPTION`,
`EMIT_EXCEPTION`, and `PRESET_NOT_FOUND`, this converts any serialization
failure in those results into `PARSE_EXCEPTION_NO_EMIT`. A lone surrogate in an
error message is enough to hit this path, changing a task error into a
success-category parse result and losing the original status. Preserve
non-success statuses here; only downgrade success statuses that need a
status-only result.
##########
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ServerProtocolIO.java:
##########
@@ -128,7 +129,21 @@ public void writeFinished(PipesResult pipesResult) throws
IOException {
} catch (IOException e) {
lastRespSerNanos = System.nanoTime() - serStart;
if (!bos.overflowed()) {
- throw e;
+ if (!(e instanceof JsonProcessingException)) {
+ throw e;
+ }
+ // the result itself cannot be encoded (the buffer is in
memory, so this is
+ // not the pipe): report it for this document rather than let
the parent
+ // count a worker crash and lose the document
+ LOG.warn("result could not be serialized; returning a
status-only result", e);
+ PipesResult.RESULT_STATUS status =
alreadyEmitted(pipesResult.status()) ?
+ pipesResult.status() :
PipesResult.RESULT_STATUS.PARSE_EXCEPTION_NO_EMIT;
+ BoundedOutputStream fallbackBos = new
BoundedOutputStream(maxIpcPayloadBytes);
+ JsonPipesIpc.toStream(new PipesResult(status,
+ "result could not be serialized: " + e.getMessage()),
fallbackBos);
Review Comment:
`fallbackBos` is still capped by `maxIpcPayloadBytes`, but the constructor
only guarantees that the static `PAYLOAD_LIMIT_EXCEEDED` frame fits. At the
minimum allowed limit, or when `e.getMessage()` is long, this status-only
message can overflow and `JsonPipesIpc.toStream` throws from inside this catch
block; `PipesServer` then handles it as a worker crash. Handle overflow here
(or bound the message) and fall back to the guaranteed-fit frame.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
