Expose the per-tag cause on PlcTagResponse, not just a bare PlcResponseCode
- Dominant language
- Java
- Stars
- 1.7k
- Forks
- 504
- Avg merge
- 10h 55m
- Merged PRs (30d)
- 43
Description
DISCLAIMER: This issue was created by Claude when playing around with some of my code.
TL/DR: If an exception is thrown internally when reading a tag, we only return INTERNAL_ERROR.
He proposes to add something like a "getException" to get access to the exception object.
I personally am a bit torn: An internal error should not occur. If it does, that's more a driver bug.
And if it happens there's generally nothing a tool could do to address it. Beyond that it leaks driver internals.
So this should be discussed and not blindly addressed.
On the one hand side in the case I ran into, it can help diagnose the core issue, however this is usually logged on the console anyway.
----------------------------
Split out of #2742, where it was the closing note — it is an API concern rather than an S7 bug,
so it seemed better tracked on its own.
## The problem
When a single tag in a multi-tag read fails, the caller gets only a `PlcResponseCode` from
`getResponseCode(tagName)`. The exception that actually caused the failure is caught inside the
driver, written to the driver's own logger, and then discarded — `PlcTagResponse` has no way to
hand it back.
That leaves an API consumer with a bare enum constant. `INTERNAL_ERROR` in particular says only
"something went wrong somewhere in the driver". For anything that puts PLC4X behind another
interface — a REST service, an OPC UA gateway, an MCP server, a UI — the cause is not merely
inconvenient to reach, it is unreachable: the process that needs to report the failure cannot get
at it, and whoever sees the error is not the person with access to the log file.
Concretely, diagnosing #2742 meant reading a tag, seeing `INTERNAL_ERROR`, then going to the
driver's log to find the `NullPointerException` — the response itself carried nothing to act on.
## Suggested shape
Something additive on `PlcTagResponse`, so nothing breaks for existing implementors:
```java
default Optional getError(String tagName) {
return Optional.empty();
}
```
Drivers that already catch a per-tag exception (the S7 driver does, in
`S7CotpConnection.decodeBindingInto`) can retain it next to the response code and return it here;
every other driver keeps the default and is unaffected. `PlcWriteResponse` has the same gap on the
encoding side and could get the same treatment.
Whether it is `Optional`, a `PlcTagError` wrapper holding code plus cause, or something
carried on the response code itself is very much open — the goal is only that the cause can reach
the caller at all. I am happy to put up a PR for whichever shape maintainers prefer.
## Workarounds, for context
The only local workaround I found is attaching an appender to the `org.apache.plc4x` logger and
correlating recent WARN/ERROR events back to the failing tag by matching the tag address in the
message text. That works for the S7 driver, which logs the address, but it depends on log wording
that is not API, breaks silently when it changes, mis-correlates under concurrency, and stops
working entirely if logging is turned down. It is a heuristic standing in for something the
response should carry.
Contributor guide
Research direction
Start with the PlcTagResponse API and the per-tag exception handling in S7CotpConnection.decodeBindingInto; also compare the analogous gap in PlcWriteResponse. The work is done when maintainers agree on a non-breaking way for callers to retrieve per-tag causes and the S7 driver exposes its caught exception through that API.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api, backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100