Defensive check for Log_OC arguments for null #338
Conversation
Log.d does not handle null, causing NPE thrown from native logger implementation. This defensive check ensures that legacy clients don't crash the system. Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com>
ezaquarii
force-pushed
the
ezaquarii/defensive-check-of-log-oc-arguments-for-null
branch
from
dedff7b to
ff1dae8
Compare
|
This fixes all issues similar to this one once and for all: |
Lint
SpotBugs (new)
SpotBugs (master)
|
|
This PR does not need a client change. |
Codecov Report
@@ Coverage Diff @@
## master #338 +/- ##
=========================================
- Coverage 25.15% 25.1% -0.05%
=========================================
Files 120 120
Lines 5267 5277 +10
Branches 695 703 +8
=========================================
Hits 1325 1325
- Misses 3782 3789 +7
- Partials 160 163 +3
|
| if (tag != null && message != null && e != null) { | ||
| Log.d(tag, message, e); | ||
| StackTraceElement[] trace = e.getStackTrace(); | ||
| appendLog(tag + ": " + message + " Exception: " + Arrays.toString(trace)); |
There was a problem hiding this comment.
Is there a reason you separated this? Could also still be inline, or?
There was a problem hiding this comment.
I'm not sure what you mean my "separation" vs "inline"?
There was a problem hiding this comment.
The logger implementation in Log_OC was extracted so we can use new logger implementation without re-writing the world. The application swaps this implementation for it's own Logger just after start, but the legacy code (other apps?) can still use the old implementation.
I suspect that something tried to log before the logger implementation was swapped, and it wrote null causing an NPE.
There was a problem hiding this comment.
Previously it was:
Arrays.toString(e.getStackTrace()))
now it is:
StackTraceElement[] trace = e.getStackTrace();
Arrays.toString(trace)
| if (tag != null && message != null && t != null) { | ||
| Log.e(tag, message, t); | ||
| StackTraceElement[] trace = t.getStackTrace(); | ||
| appendLog(tag + ": " + message + " Exception: " + Arrays.toString(trace)); |
|
@ezaquarii thanks again for your PR and finding the cause of the problem 🎉 |
|
Line became too long and linter complained. :)
|
delete-merged-branch
bot
deleted the
ezaquarii/defensive-check-of-log-oc-arguments-for-null
branch
|
/backport to stable-1.6 |
|
backport to stable-1.6 in #341 |
Log.d does not handle null, causing NPE thrown from
native logger implementation. This defensive check
ensures that legacy clients don't crash the system.
Signed-off-by: Chris Narkiewicz hello@ezaquarii.com