Patching error message verbosity#27692
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Khairajani
changed the title
Khairajani
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
🔴 Playwright Results — 5 failure(s), 7 flaky✅ 3985 passed · ❌ 5 failed · 🟡 7 flaky · ⏭️ 86 skipped
Genuine Failures (failed on all attempts)❌
|
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
Keep the patch_body in the warning log; the main side dropped it.
Avoid leaking patch values (descriptions, sample data, tags) in WARNING logs. The Jackson server message in 'Reason:' still carries the offending value for the failing field, which is enough to debug deserialization errors without dumping every field being changed.
Code Review ✅ Approved 2 resolved / 2 findingsReplaces full body dumps with op:path summaries in patch failure logs to prevent sensitive data exposure, and resolves lingering merge conflict markers in patch_mixin.py. No issues found. ✅ 2 resolved✅ Bug: Unresolved merge conflict markers left in source code
✅ Security: Patch body in logs/errors may expose sensitive fields
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Khairajani
changed the title
1 participant
Summary
Improves error message verbosity for patch operations so failures are easier to diagnose, without leaking entity values into logs.
Changes
patch_mixin.py— When a patch call fails, the warning /RuntimeErrornow includes a compact summary of the JSON Patch (op count andop:pathlist) alongside the underlying error. A new_summarize_patchhelper builds the summary and intentionally excludes patch values, since they may carry descriptions, sample data, or tag content that should not appear in logs. Also tightens control flow in theexceptblock (drops the redundantelseafterreturn).JsonUtils.java—applyPatchnow reports the target class name and the underlying exception message (Failed to convert JsonValue to <ClassName>: <message>) instead of the opaqueFailed to convert JsonValue to target class.Test plan
Patch ops: N op(s) [op:path, ...]with no values.applyPatchfailure on the Java side and confirm the new error string includes the target class and root cause message.