Better Error message when CouchDB is unavailable#69
Better Error message when CouchDB is unavailable#69vaibhav45sktech wants to merge 4 commits intodbpedia:masterfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduce a CouchDB-down safe SPARQL fallback and make Explorer DB initialization resilient: Application initializes each DB in separate guarded blocks, uses Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Helper as Helper
participant ExplorerDB as ExplorerDB (CouchDB)
participant SPARQL as SPARQL
App->>Helper: initialize()
Helper->>ExplorerDB: attempt connect
alt ExplorerDB available
ExplorerDB-->>Helper: connected
Helper->>SPARQL: new SPARQL(explorerDB)
SPARQL-->>Helper: operational instance
else ExplorerDB unavailable
ExplorerDB-->>Helper: error
Helper->>SPARQL: SPARQL.disabled()
SPARQL-->>Helper: inert fallback instance
end
Note over Helper,SPARQL: Runtime requests use SPARQL instance (real or disabled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/Application.java`:
- Around line 125-141: The logger message is misleading and the try/catch causes
partial initialization; change to initialize chatDB, feedbackDB, and explorerDB
in separate try/catch blocks (or loop through names) so each call to
cloudantClient.database(chatDBName, true),
cloudantClient.database(feedbackDBName, true), and
cloudantClient.database(explorerDBName, true) is attempted independently; on
each failure call logger.error with a DB-specific message and the caught
exception (e.g., "Failed to open chatDB", "Failed to open feedbackDB", "Failed
to open explorerDB") and ensure downstream code (where sparql is set from
explorerDB) checks for null or uses SPARQL.disabled() as before so partial
successes are explicit and logged.
In `@src/main/java/chatbot/lib/api/SPARQL.java`:
- Around line 50-78: The disabled() SPARQL factory returns an anonymous subclass
but does not override getDisambiguatedEntities(), which allows calls to fall
through into getEntities → processEntityInformation → getRelevantProperties and
ultimately cause an NPE at explorerDB.getViewRequestBuilder(); add an override
for getDisambiguatedEntities(String uri) in the anonymous class returned by
SPARQL.disabled() that returns a safe empty list (e.g., new ArrayList<>() or
Collections.emptyList()) or an appropriate empty-response value so callers
(including ResponseInfo.nextPage(sparql)) never attempt to access explorerDB
when the SPARQL instance is disabled.
🧹 Nitpick comments (1)
src/main/java/chatbot/Application.java (1)
146-146: SimplifyArrays.asListcall.The explicit
new String[]is unnecessary with varargs.✨ Suggested simplification
- List<String> wordsToIgnore = Arrays.asList(new String[] { "nlp", "merkel" }); + List<String> wordsToIgnore = Arrays.asList("nlp", "merkel");
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/Application.java`:
- Around line 49-53: The log in initializeFBMessengerSendClient currently prints
the pageAccessToken; remove the secret from logs by changing the logger call to
a non-sensitive message (e.g., "Initializing MessengerSendClient") and do not
interpolate pageAccessToken anywhere; keep returning
MessengerPlatform.newSendClientBuilder(pageAccessToken).build() but ensure
pageAccessToken is never logged or included in formatted messages.
In `@src/main/java/chatbot/lib/api/SPARQL.java`:
- Around line 92-116: stripWikiepdiaContent currently miscomputes and trusts
index arithmetic and can return the same string (causing an infinite loop in
processWikipediaAbstract); fix it by (1) computing indexEnd via int indexEnd =
text.indexOf(")", indexStart); if indexEnd == -1 then return text unchanged (do
not add 2 or use +2), otherwise set indexEnd = Math.min(indexEnd + 1,
text.length()) and remove the substring between indexStart and indexEnd
(inclusive) safely; (2) for the disambiguation branch (when indexStart == 0)
compute int disIndex = text.lastIndexOf("(disambiguation).)"), verify disIndex
!= -1 before slicing and ensure the computed end index does not exceed
text.length(); and (3) in processWikipediaAbstract, after each call to
stripWikiepdiaContent check whether the returned string changed (e.g., compare
before/after) and break the loop if unchanged to prevent infinite looping on
malformed input.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/lib/api/SPARQL.java`:
- Around line 200-207: The loop that populates the LinkedHashMap uses keyArray
and valueArray without checking lengths, risking ArrayIndexOutOfBoundsException;
update the code around keyArray/valueArray (derived from result.get("values")
and result.get("value_labels")) to iterate only up to Math.min(keyArray.length,
valueArray.length), skip or log any mismatched extras (e.g., using
process/logger or a warning), still call Utility.convertDBpediaToWikipediaURL
for each safe index and then call field.setValues(map); this ensures no
out-of-bounds access while preserving existing behavior.
- Around line 391-397: The code in SPARQL.java calls results.next() on the
Iterator<QuerySolution> returned by queryExecution.execSelect() without checking
results.hasNext(), which can throw NoSuchElementException when the query yields
no rows; update the block around execSelect()/results to check results.hasNext()
before calling next(), handle the empty-case by returning a sensible default or
throwing a controlled exception, and ensure queryExecution.close() still runs in
the finally block; reference the Iterator<QuerySolution> results, the call to
queryExecution.execSelect(), and the label variable when adding the conditional
handling.
|
Greetings @RicardoUsbeck , could you kindly review my pr whenever available . |
Summary
Fixes CouchDB unavailability crashes and improves error handling when database is down.
Key Changes
isExplorerDBAvailable()disabled()method with all overrides, fix bounds exception in text processingImpact
Resolves #44
closes #44
Summary by CodeRabbit
New Features
Bug Fixes
Chores