[NAE-2381] Log in RestResponseExceptionHandler

[renczesstefan]

Description

Added additional logging for RestResponseExceptionHandler

Implements NAE-2381

Dependencies

No new dependencies were introduced

Third party dependencies

No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

This was tested manually.

Test Configuration

Name	Tested on
OS	macOS Tahoe 26.2
Runtime	Java 21
Dependency Manager	Maven 3.9.9n
Framework version	Spring Boot 3.4.4
Run parameters
Other configuration

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My changes have been checked, personally or remotely, with @machacjozef
• I have commented my code, particularly in hard-to-understand areas
• I have resolved all conflicts with the target branch of the PR
• I have updated and synced my code with the target branch
• I have added tests that prove my fix is effective or that my feature works
• New and existing tests pass locally with my changes:
  • Lint test
  • Unit tests
  • Integration tests
• I have checked my contribution with code analysis tools:
  • SonarCloud
  • Snyk
• I have made corresponding changes to the documentation:
  • Developer documentation
  • User Guides
  • Migration Guides

Summary by CodeRabbit

• Chores
  • Safer handling of HTTP response serialization errors to prevent failures during error reporting and avoid secondary crashes in edge cases.
  • Improved logging: error conditions are recorded at higher severity and include contextual identifiers when available to help diagnostics, while preserving existing error responses and behavior for callers.

[coderabbitai]

Walkthrough

handleHttpMessageNotWritable now validates the exception cause and inspects JsonMappingException path safely: if the path length > 3 it extracts the last field and the element three steps back as possible Field and Case, logs debug with IDs/value on success, otherwise logs JSON write failure; unexpected errors are logged as error. Superclass handling is preserved.

Changes

Cohort / File(s)	Summary
Exception handler update application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java	Guarded, type-safe traversal of JsonMappingException path (check path length > 3), extract last field and element three steps back when present, log debug with field/case info if casts succeed, otherwise log JSON write failure; changed unrecognized-exception logging from WARN to ERROR; retains delegation to superclass handler.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug, logging

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title references a ticket (NAE-2381) and mentions the affected component (RestResponseExceptionHandler), but the actual change—improved logging with type-safe path validation—is not conveyed. The title is too vague about what logging was added.	Consider a more specific title such as 'Add defensive logging in RestResponseExceptionHandler' or 'Improve error handling and logging in RestResponseExceptionHandler' to better convey the nature of the changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java (1)

33-33: ⚠️ Potential issue | 🟠 Major

Potential PII leak via from.getValue() at ERROR log level.

from.getValue() logs the raw value of a form field, which can contain arbitrary user-submitted data — names, addresses, health records, financial figures, etc. Writing this at ERROR level means it ends up in persistent application logs, shipped to log aggregators, and potentially retained long-term, which conflicts with GDPR/CCPA data-minimisation and logging-of-personal-data obligations.

Since this PR is explicitly a logging review pass, this is the right time to address it. Consider one of:

1. Omit the value entirely and log only the field ID (which is already sufficient to locate the problem).
2. Replace the value with a type indicator or a hash (e.g., from.getFieldType(), Objects.hashCode(from.getValue())).

🛡️ Proposed fix — remove raw field value from the log line

-log.error("[{}] Could not parse value of field [{}], value [{}]", useCase.getStringId(), from.getStringId(), from.getValue());
+log.error("[{}] Could not serialize field [{}] (type: {})", useCase.getStringId(), from.getStringId(), from.getClass().getSimpleName());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java`
at line 33, The ERROR log in RestResponseExceptionHandler currently includes the
raw field value via from.getValue(), which may leak PII; update the log in the
method where useCase and from are available (the handler method in class
RestResponseExceptionHandler) to stop logging from.getValue() directly — either
remove the value from the message and only log useCase.getStringId() and
from.getStringId(), or replace it with a non-sensitive indicator such as
from.getFieldType() or Objects.hashCode(from.getValue()) to preserve
traceability without exposing user data.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java`:
- Line 33: The ERROR log in RestResponseExceptionHandler currently includes the
raw field value via from.getValue(), which may leak PII; update the log in the
method where useCase and from are available (the handler method in class
RestResponseExceptionHandler) to stop logging from.getValue() directly — either
remove the value from the message and only log useCase.getStringId() and
from.getStringId(), or replace it with a non-sensitive indicator such as
from.getFieldType() or Objects.hashCode(from.getValue()) to preserve
traceability without exposing user data.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java`:
- Line 32: The current error log in RestResponseExceptionHandler logs the raw
field value via from.getValue(), which may expose PII; change the log at the
location that calls log.error("[{}] Could not parse value of field [{}], value
[{}]", useCase.getStringId(), from.getStringId(), from.getValue()) to omit the
raw value and instead log only the field id and type (e.g., from.getStringId()
and from.getType()), or a clearly redacted/truncated placeholder (e.g.,
"<REDACTED>" or first N chars + "..."); ensure the log message is updated
accordingly so no sensitive data from from.getValue() is emitted while
preserving useful context for debugging.
- Around line 36-38: The catch block in RestResponseExceptionHandler currently
logs only the inner exception variable (e) and drops the original
HttpMessageNotWritableException context; update the catch to include the
original exception's message/instance in the log so both exceptions appear in
the record (for example include the outer exception's getMessage() or the outer
exception object as context in the log call), locating the change in the catch
(Exception e) block inside RestResponseExceptionHandler that handles
HttpMessageNotWritableException. Ensure the log message clearly names the
operation (e.g., handling HttpMessageNotWritableException) and passes both the
original exception info and the inner exception so traces are correlated in
aggregated logs.
- Line 28: Replace the Java-21-only call path.getLast() in
RestResponseExceptionHandler.java with a Java‑11 compatible indexed access;
specifically, when obtaining the last JsonMappingException.Reference (variable
fieldReference), use path.get(path.size() - 1) instead of path.getLast() so the
code runs on the project's Java 11+ baseline.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java`:
- Around line 36-38: Replace the magic literal 3 with a named constant and add a
brief explanatory comment about the expected Jackson path structure; for example
introduce a constant like CASE_FROM_INDEX_OFFSET = 3 in
RestResponseExceptionHandler and use it when checking path.size() and accessing
path.get(path.size() - CASE_FROM_INDEX_OFFSET), and add a one-line comment near
that constant or next to the code explaining the assumed path order (e.g.,
"expected path: Case → dataSet → <key> → Field") so future maintainers
understand why we look three positions back when computing fieldFrom/caseFrom.
- Around line 36-50: Refactor the conditional in RestResponseExceptionHandler to
remove duplicated log.error blocks by first checking path.size() > 3 then
obtaining Object fieldFrom = path.getLast().getFrom() and Object caseFrom =
path.get(path.size() - 3).getFrom(), and only when both are the expected types
(use instanceof Field field && caseFrom instanceof Case useCase) call
log.debug(...) with useCase.getStringId(), field.getStringId(),
field.getValue(), jme.getPathReference(); otherwise call a single
log.error("JSON write failed: {} | path={}", jme.getOriginalMessage(),
jme.getPathReference(), jme) once; ensure the branch structure uses
short-circuiting so getLast() and get(...) are only called when path.size() > 3
and avoid duplicating the log.error call.

---

Duplicate comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/RestResponseExceptionHandler.java`:
- Line 37: The code uses path.getLast() (in RestResponseExceptionHandler.java)
which requires Java 21; verify and lock the project Java baseline to 21 by
ensuring the pom.xml and build plugins (maven-compiler-plugin/toolchains or
similar) explicitly set source/target (or java.version property) to 21 and
update CI/build configs if needed so the project cannot regress to an older Java
version; if you must support older JDKs, replace path.getLast() with a
compatible alternative (e.g., retrieving the last element via
path.get(path.size()-1)) and adjust unit/compile settings accordingly.
- Around line 41-42: The debug log in RestResponseExceptionHandler currently
prints raw field values via field.getValue(), which may expose PII; change the
log call in the handling of JSONMappingException (the log.debug that references
useCase.getStringId(), field.getStringId(), field.getValue(),
jme.getPathReference()) to avoid logging the raw value—either remove
field.getValue() entirely or replace it with a redacted/masked representation
(e.g., "<redacted>" or a boolean/length/type indicator) so you still have
context (useCase.getStringId(), field.getStringId(), jme.getPathReference())
without exposing sensitive data.
- Around line 52-54: The catch block in RestResponseExceptionHandler currently
logs only the inner exception variable 'e' and loses the original 'exception'
context; update the catch to include the original exception's message
(exception.getMessage()) in the log call so both the original exception text and
the caught stacktrace 'e' are recorded (e.g., include exception.getMessage() as
part of the log message while still passing 'e' as the throwable) so correlation
between 'exception' and 'e' is preserved.