Diagnose #5759: Add context to AcquisitionsDialog btnDepod NPE#8888
Conversation
There was a problem hiding this comment.
Pull request overview
Adds diagnostic context around the btnDepod ActionListener in AcquisitionsDialog to help identify the root cause of issue #5759 (NPE from part.getMissingPart() becoming null between panel construction and button click).
Changes:
- Adds a null-check for
part.getMissingPart()inside thebtnDepodclick handler. - Logs detailed context (part name/id, omniPodCount, missingCount, isOmniPoddable) via
MMLoggerfor Sentry visibility. - Rethrows a
NullPointerExceptionto preserve the existing global uncaught-exception handling behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8888 +/- ##
=========================================
Coverage 13.81% 13.82%
- Complexity 8170 8176 +6
=========================================
Files 1282 1282
Lines 167371 167383 +12
Branches 25231 25232 +1
=========================================
+ Hits 23119 23133 +14
+ Misses 141925 141919 -6
- Partials 2327 2331 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Don't need to re-throw. The Uncaught Handler will also submit to Sentry (if enabled). If it is something that can be dealt with and moved on, do so. As in if we can safely recover from it and move on, put up a dialog stating something changed and changes unsaved/try again, capture the error, report it, and move on. No need to log it twice. |
Sounds good, I'll handle it instead of throwing. |
Updated with your feedback. |
rjhancock
left a comment
There was a problem hiding this comment.
Sanity Check Complete.
|
@rjhancock Same, I can't merge :) |
|
I only did a sanity check on the code. I'll let someone else test it and merge. |
| logger.error(ise, | ||
| "btnDepod clicked but part.getMissingPart() returned null. " + | ||
| "part={} (id={}), omniPodCount={}, missingCount={}, isOmniPoddable={}", | ||
| part.getName(), | ||
| part.getId(), | ||
| partCountInfo.getOmniPodCount(), | ||
| partCountInfo.getMissingCount(), | ||
| part.isOmniPoddable()); |
There was a problem hiding this comment.
You have this wrong, that's correct as there are 5 {}
Related to #5759. Does not fix the bug
The
btnDepodlistener inAcquisitionsDialogcallspart.getMissingPart().setOmniPodded(true)with no null check. The button's visibility guard runs only at panel build time, so a state change before click can produce the reported NPE. The Sentry trace alone doesn't tell us which part or state caused it.This PR null-checks
getMissingPart(), logs an error withpart name/id,omniPodCount,missingCount, andisOmniPoddable(forwarded to Sentry viaMMLogger), then rethrows so existing error handling is unchanged. Next occurrence will give us actionable context for a real fix.