[
https://issues.apache.org/jira/browse/TIKA-4846?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108929#comment-18108929
]
ASF GitHub Bot commented on TIKA-4846:
--------------------------------------
Copilot commented on code in PR #3082:
URL: https://github.com/apache/tika/pull/3082#discussion_r3874551565
##########
tika-pipes/tika-pipes-plugins/tika-pipes-file-system/src/test/java/org/apache/tika/pipes/fs/ConfigExamplesTest.java:
##########
@@ -42,4 +54,31 @@ public void testFileSystemEmitterConfig() throws Exception {
public void testFileSystemPipelineConfig() throws Exception {
loadAndValidate("file-system-pipeline.json");
}
+
+ @Test
+ public void testFileSystemJsonlReporterConfig() throws Exception {
+ loadAndValidate("file-system-jsonl-reporter.json");
+
+ JsonNode inner =
innerComponent(readExample("file-system-jsonl-reporter.json"),
+ "pipes-reporters", null, "file-system-jsonl-reporter");
+ FileSystemJsonlReporterConfig config =
FileSystemJsonlReporterConfig.load(inner.toString());
+ assertEquals("/var/log/tika/pipes-audit.jsonl",
config.path().toString());
Review Comment:
This assertion hard-codes the UNIX path string. On Windows,
`Path.toString()` will likely render with backslashes (e.g., `\var\log\...`),
making this test OS-dependent even though the example JSON is meant for
documentation only. Normalize separators (or assert on the raw JSON string) to
keep the test portable.
##########
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/async/AsyncProcessor.java:
##########
@@ -142,6 +145,9 @@ private AsyncProcessor(Path tikaConfigPath, PipesIterator
pipesIterator,
checkActive();
} catch (InterruptedException e) {
return WATCHER_FUTURE_CODE;
+ } catch (RuntimeException e) {
+ // already latched in failure; rethrowing would make
this future a second one
+ return WATCHER_FUTURE_CODE;
}
Review Comment:
The watcher thread catches *all* `RuntimeException`s from `checkActive()`
and exits quietly. That will also swallow unexpected runtime failures (e.g., an
IllegalArgumentException for an unknown future code), making the processor
continue without its watcher and hiding the root cause. Only suppress the
RuntimeException when the failure has already been latched; otherwise rethrow
so the error is surfaced.
##########
tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/async/AsyncProcessor.java:
##########
@@ -343,8 +351,10 @@ public synchronized boolean checkActive() throws
InterruptedException {
throw new IllegalArgumentException("Don't recognize
this future code: " + i);
}
} catch (ExecutionException e) {
- LOG.error("execution exception", e);
- this.pipesReporter.error(e);
+ if (failure.compareAndSet(null, e)) {
+ LOG.error("execution exception", e);
+ this.pipesReporter.error(e);
+ }
Review Comment:
If `pipesReporter.error(e)` throws, it will escape this catch block and mask
the original `ExecutionException` from the worker/emitter. That makes
diagnosing the real failure harder and is inconsistent with the later logic
that avoids reporter exceptions masking the primary stop reason. Consider
catching reporter RuntimeExceptions here and suppressing them onto the original
`ExecutionException` before rethrowing.
> Add jsonl status reporter
> -------------------------
>
> Key: TIKA-4846
> URL: https://issues.apache.org/jira/browse/TIKA-4846
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Minor
>
> For our eval runs, it would be helpful to have a jsonl status reporter to
> record causes of crashes. Currently, all we know is that there's no .json for
> a given input file. We can grep the logs, but that's wasteful.
> On a follow up ticket, we can add a reader of this for tika-eval.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
