[
https://issues.apache.org/jira/browse/TIKA-4897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18116036#comment-18116036
]
ASF GitHub Bot commented on TIKA-4897:
--------------------------------------
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.
> Handle unpaired surrogates more robustly
> ----------------------------------------
>
> Key: TIKA-4897
> URL: https://issues.apache.org/jira/browse/TIKA-4897
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Minor
>
> An unpaired surrogate causes less than ideal behavior during serialization.
> We should handle this more robustly.
>
> After a bit of agentic digging, this is a smile-only issue, so it only hits
> in IPC between tika-pipes client and server. This does not affect our
> standard json serdes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
