fix(OFJAVA-018): 2 review findings in CacheServiceAbs.java - #84
fix(OFJAVA-018): 2 review findings in CacheServiceAbs.java#84flamingo[bot] wants to merge 1 commit into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| put(cachePath, cacheKey, response); | ||
| return Optional.of(response); | ||
| } catch (Exception e) { | ||
| log.error("Error fetching data: {}", e.getMessage()); | ||
| log.error("Error fetching data", e); | ||
| return Optional.empty(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🦩 🔴 Exception swallowed with message-only logging instead of full exception object
In doHttpCall (catch block around line 155), changed log.error("Error fetching data: {}", e.getMessage()); to log.error("Error fetching data", e);, passing the exception object so the stack trace is preserved.
🤖 Prompt for AI agents
In backend/src/main/java/cx/flamingo/analysis/cache/CacheServiceAbs.java around line 155, review and complete this code-review fix: Exception swallowed with message-only logging instead of full exception object.
What the draft fix changed: In doHttpCall (catch block around line 155), changed `log.error("Error fetching data: {}", e.getMessage());` to `log.error("Error fetching data", e);`, passing the exception object so the stack trace is preserved.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| } | ||
| return isStale; | ||
| } catch (Throwable e) { | ||
| log.error("Error checking cache entry age: {}", e.getMessage()); |
There was a problem hiding this comment.
🦩 🔴 Cache-entry-age check logs only exception message, discarding stack trace
In isCacheEntryStale (catch block around line 74), changed log.error("Error checking cache entry age: {}", e.getMessage()); to log.error("Error checking cache entry age", e);, passing the exception object so the stack trace is preserved.
🤖 Prompt for AI agents
In backend/src/main/java/cx/flamingo/analysis/cache/CacheServiceAbs.java around line 74, review and complete this code-review fix: Cache-entry-age check logs only exception message, discarding stack trace.
What the draft fix changed: In isCacheEntryStale (catch block around line 74), changed `log.error("Error checking cache entry age: {}", e.getMessage());` to `log.error("Error checking cache entry age", e);`, passing the exception object so the stack trace is preserved.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
Closes 2 review findings in
backend/src/main/java/cx/flamingo/analysis/cache/CacheServiceAbs.java.Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
backend/src/main/java/cx/flamingo/analysis/cache/CacheServiceAbs.java:155backend/src/main/java/cx/flamingo/analysis/cache/CacheServiceAbs.java:74What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
89424001-8db6-4ce6-8840-ab853de69d56Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.