fskorgen opened a new issue, #8125:
URL: https://github.com/apache/hop/issues/8125
### Apache Hop version?
2.19
### Java version?
21
### Operating system
Windows
### What happened?
A transform with error handling configured writes one ERROR line to the
pipeline log for every
rejected row. The rows reach the error hop and the pipeline finishes
successfully — the log lines
are the only effect. Up to 2.18 nothing was logged.
Error handling is the normal way to route expected misses: a Database lookup
that does not find a
key, a Data validator that rejects a value, a REST call that returns 404. A
pipeline built that way
now produces one ERROR per rejected row, so a run over a few hundred
thousand rows fills the log
with errors from a pipeline that did exactly what it was designed to do. Log
monitoring on ERROR
becomes unusable, since real failures no longer stand out.
Seen in production after upgrading 2.18 → 2.19: a nightly run floods the log
with `No lookup found`
from a single Database lookup that has an error hop, on a pipeline whose
behaviour did not change.
### Cause
`BaseTransform.handlePutError()`, in the branch where the error row **was**
delivered to the error
row set:
```java
if (errorRowSet != null) {
while (!errorRowSet.putRow(errorRowMeta, errorRowData)) {
if (isStopped()) {
break;
}
}
incrementLinesRejected();
if (isLoggingErrorDescriptions() && !Utils.isEmpty(errorDescriptions)) {
logError(errorDescriptions);
}
} else if (transformErrorMeta.isEnabled()) {
...
setErrors(1);
stopAll();
}
```
The `logError` came in with 73db87e548 (#7323, fixing #7303 "REST client
silent failure when error
hop is disabled"). The silent failure that issue describes is the `else`
branch — an enabled error
hop with no reachable target — and that branch logs and stops the pipeline
on its own. Logging in
the first branch as well turns every successfully handled row into an ERROR.
#7570 recognised the volume problem and added
`isLoggingErrorDescriptions()`, but only Data
validator overrides it, and only when the new "suppress error log" option is
ticked. Every other
transform still logs.
### Secondary: what Database lookup puts in that log line
Database lookup is where this surfaced, and it shows a second problem — the
description the engine
now prints is not written to be read:
```java
putError(getInputRowMeta(), row, 1L, "No lookup found", null, "DBL001");
```
`DatabaseLookup.java:147`. Three things about that call:
- **The text is a hardcoded English literal**, not a message key, so it is
untranslated in every
locale. The transform already has a bundle key for the same event, used
one line below for the
row-level log: `DatabaseLookup.Log.NoResultsFoundAfterLookup`.
- **It says nothing about which lookup missed.** No key value, no table, no
return fields. A hundred
thousand identical `No lookup found` lines carry as much information as
one.
- **The field names argument is `null`**, so the error hop's error-fields
column is empty as well —
the same gap in the error stream, not just in the log.
This was harmless while the description was only data on the error hop. Now
that every rejected row
is logged, the description is user-facing text and should read like it. The
same applies to the
other transforms that pass literals here: of the 168 `putError(` calls under
`plugins/transforms`,
none uses `BaseMessages`.
### Steps to reproduce
1. Pipeline: Data grid → Database lookup → error hop to a Dummy.
2. Feed keys that do not exist in the lookup table.
3. Run.
**Expected:** rejected rows go down the error hop, log stays clean (2.18
behaviour).
**Actual:** one `ERROR ... No lookup found` per rejected row; pipeline
result is still success.
### Suggested fix
Keep the ERROR for the unhandled case, and log at debug when the row was
delivered:
```java
incrementLinesRejected();
if (isLoggingErrorDescriptions() && !Utils.isEmpty(errorDescriptions)) {
logDebug(errorDescriptions);
}
```
The description stays available at Debug log level, #7303 keeps its fix in
the `else` branch, and
the Data validator option added by #7570 still works.
Independently of that, `DatabaseLookup` should pass a message key instead of
the literal, and
include the key values that missed — the description is now log text, not
just a column on the
error hop.
If the ERROR line is wanted for transforms that reject rarely, the
alternative is to flip the
default — `isLoggingErrorDescriptions()` returning false in `BaseTransform`,
overridden to true
where a rejected row really is exceptional. Either way the current default
of logging every handled
row is what breaks pipelines built around error handling.
### Issue Priority
Priority: 2
### Issue Component
Component: Transforms
--
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]
