Fix clearer error message when CouchDB is unavailable#66
Fix clearer error message when CouchDB is unavailable#66Dev10-sys wants to merge 3 commits intodbpedia:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application/Helper
participant DB as ExplorerDB
participant SP as SPARQL
App->>DB: check explorerDB != null
alt explorerDB available
App->>SP: new SPARQL(explorerDB)
SP->>App: operational SPARQL instance
else explorerDB missing
App->>SP: SPARQL.disabled()
SP->>App: safe-disabled SPARQL instance
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/chatbot/Application.java (1)
125-147: Error message may be misleading when chatDB or feedbackDB fails.The catch block logs "Explorer properties will not be loaded" but the exception could occur on any of the three database initializations (lines 126-128). If
chatDBorfeedbackDBfails, the message is inaccurate and users may not realize those databases are also unavailable.Consider either:
- Initializing each database in its own try-catch with specific error messages, or
- Updating the message to reflect all three databases could be affected.
🔧 Suggested fix with individual error handling
try { chatDB = cloudantClient.database(chatDBName, true); + } catch (Exception e) { + logger.error("CouchDB / Cloudant chatDB is unavailable.", e); + } + try { feedbackDB = cloudantClient.database(feedbackDBName, true); + } catch (Exception e) { + logger.error("CouchDB / Cloudant feedbackDB is unavailable.", e); + } + try { explorerDB = cloudantClient.database(explorerDBName, true); - } catch (Exception e) { - logger.error("CouchDB / Cloudant is unavailable. Explorer properties will not be loaded.", e); - } finally { + } catch (Exception e) { + logger.error("CouchDB / Cloudant explorerDB is unavailable. Explorer properties will not be loaded.", e); + } + this.tmdbApiKey = tmdbApiKey; this.wolframRepository = wolframRepository; // ... rest of initialization - }
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/Application.java`:
- Line 139: Multiple call sites assume getSparql() is non-null but Application
now sets sparql to null when explorerDB is absent; update all callers (e.g.,
TemplateHandler where helper.getSparql().getEntityInformation(...) is used and
LocationHandler where helper.getSparql().getLabel(uri) is used) to first check
if helper.getSparql() == null and handle that case (early return, error
response, or fallback behavior) instead of dereferencing, or alternatively
change getSparql() to throw an explicit IllegalStateException when sparql is
null and update callers to catch/propagate that—ensure every usage of
getSparql() is guarded to prevent NPEs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/lib/api/SPARQL.java`:
- Around line 45-72: The disabled SPARQL instance lacks an override for
getEntitiesByURIs(), causing a NullPointerException when OptionsTemplateHandler
calls it (because getEntitiesByURIs → getRelevantProperties →
explorerDB.getViewRequestBuilder uses explorerDB which is null). Fix by
overriding getEntitiesByURIs() in the disabled() anonymous SPARQL to return a
safe empty/placeholder ResponseData or list (matching the method signature) and
ensure any helper methods that would touch explorerDB (e.g.,
getRelevantProperties) are similarly short-circuited in this disabled instance;
reference getEntitiesByURIs, getRelevantProperties,
explorerDB.getViewRequestBuilder and OptionsTemplateHandler to locate the call
sites.
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 90-96: The method stripWikiepdiaContent can throw
StringIndexOutOfBounds because it adds +2 to the closing-paren index before
checking; change the logic in stripWikiepdiaContent to (1) find indexStart with
indexOf("(") and proceed only if indexStart >= 0, (2) find the raw closing index
via indexOf(")", indexStart) and verify it's not -1, (3) compute indexEnd =
rawClose + 2 but clamp it to text.length() (or otherwise ensure indexEnd <=
text.length()) before calling substring/replace, and (4) only call
text.substring(indexStart, indexEnd) when both indices are within bounds; this
prevents out-of-range accesses while preserving the intended trimming behavior.
- Around line 45-76: The disabled() SPARQL stub lacks an override for
getDisambiguatedEntities(), which allows the pagination path
(ResponseInfo.nextPage() used by OptionsTemplateHandler) to call
getRelevantProperties() and dereference explorerDB causing an NPE; add an
override of getDisambiguatedEntities(String uri, int page, int pageSize) in the
disabled() anonymous subclass that returns an empty/placeholder pagination-safe
result (e.g., empty list or ResponseData wrapper consistent with the method's
contract) and ensure it does not access explorerDB or call
getRelevantProperties(), mirroring the safe-return pattern used in
getEntitiesByURIs(), getEntityInformation(), and getLabel.
| /** | ||
| * Safe disabled SPARQL instance used when CouchDB is unavailable. | ||
| * Prevents NullPointerExceptions across the application. | ||
| */ | ||
| public static SPARQL disabled() { | ||
| return new SPARQL(null) { | ||
|
|
||
| @Override | ||
| public ResponseData getEntityInformation(String uri) { | ||
| ResponseData response = new ResponseData(); | ||
| response.setTitle("Explorer database unavailable"); | ||
| response.setText( | ||
| "The DBpedia explorer database is currently unavailable. " + | ||
| "Please try again later."); | ||
| return response; | ||
| } | ||
|
|
||
| @Override | ||
| public String getLabel(String uri) { | ||
| return "Explorer database unavailable"; | ||
| } | ||
|
|
||
| @Override | ||
| public int isDisambiguationPage(String uri) { | ||
| return 0; | ||
| } | ||
|
|
||
| @Override | ||
| public ArrayList<ResponseData> getEntitiesByURIs(String uris) { | ||
| // CouchDB down → return safe empty result | ||
| return new ArrayList<>(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full structure of SPARQL.java to understand the disabled() method and call chains
rg -n --type java -A 100 'public static SPARQL disabled' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 3667
🏁 Script executed:
# Check for getDisambiguatedEntities call sites in the codebase
rg -n --type java '\bgetDisambiguatedEntities\s*\('Repository: dbpedia/chatbot
Length of output: 513
🏁 Script executed:
# Look at the processEntityInformation and getRelevantProperties methods
rg -n --type java -A 10 '(processEntityInformation|getRelevantProperties)\s*\(' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 2789
🏁 Script executed:
# Check if explorerDB is accessed in the chain and where it's dereferenced
rg -n --type java -B 2 -A 5 'explorerDB\.' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 500
🏁 Script executed:
# Check NLHandler.java around line 127 to see if isDisambiguationPage is checked first
rg -n --type java -B 10 -A 5 'getDisambiguatedEntities\(uri, 0, ResponseData.MAX_DATA_SIZE\)' src/main/java/chatbot/lib/handlers/NLHandler.javaRepository: dbpedia/chatbot
Length of output: 1006
🏁 Script executed:
# Get the full getDisambiguatedEntities method implementation
rg -n --type java -A 30 'public ArrayList<ResponseData> getDisambiguatedEntities\(String uri, int offset, int limit\)' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 1526
🏁 Script executed:
# Check if getRelevantProperties has any null check for explorerDB
rg -n --type java -B 3 -A 20 'private List<ResponseData.Field> getRelevantProperties\(' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 1295
🏁 Script executed:
# Check the call site at line 482 in SPARQL.java
rg -n --type java -B 10 -A 5 'getDisambiguatedEntities\(uri, offset, limit\)' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 525
🏁 Script executed:
# Get the full getEntities method to trace the complete chain
rg -n --type java -A 25 'private ArrayList<ResponseData> getEntities\(' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 1307
🏁 Script executed:
# Verify the guard flow in NLHandler - check line 115-116 to confirm isDisambiguationPage call
rg -n --type java -B 15 'if\(count == 0\)' src/main/java/chatbot/lib/handlers/NLHandler.java | head -30Repository: dbpedia/chatbot
Length of output: 847
🏁 Script executed:
# Let me check if processEntityInformation always calls getRelevantProperties or if there are conditions
rg -n --type java -B 5 -A 50 'private ResponseData processEntityInformation' src/main/java/chatbot/lib/api/SPARQL.java | head -80Repository: dbpedia/chatbot
Length of output: 2641
🏁 Script executed:
# Check the full flow to understand when getRelevantProperties is called
sed -n '221,280p' src/main/java/chatbot/lib/api/SPARQL.javaRepository: dbpedia/chatbot
Length of output: 2746
🏁 Script executed:
# Let me check all call sites of getDisambiguatedEntities more carefully
rg -n --type java -B 8 'getDisambiguatedEntities' src/main/java/chatbot/lib/handlers/NLHandler.javaRepository: dbpedia/chatbot
Length of output: 778
🏁 Script executed:
# Also check if ResponseInfo.nextPage() is called directly without prior guard
rg -n --type java -B 5 -A 3 'nextPage\('Repository: dbpedia/chatbot
Length of output: 1812
Override getDisambiguatedEntities() in disabled() to prevent NPE on pagination.
getDisambiguatedEntities() calls getRelevantProperties() which dereferences explorerDB without a null check (line 151). While the primary call in NLHandler (line 127) is protected by the isDisambiguationPage() guard, the pagination path via ResponseInfo.nextPage() (OptionsTemplateHandler line 119) can bypass this guard if a disabled instance is used after initial response generation, causing an NPE.
Suggested override
return new SPARQL(null) {
+ `@Override`
+ public ArrayList<ResponseData> getDisambiguatedEntities(String uri, int offset, int limit) {
+ return new ArrayList<>();
+ }🤖 Prompt for AI Agents
In `@src/main/java/chatbot/lib/api/SPARQL.java` around lines 45 - 76, The
disabled() SPARQL stub lacks an override for getDisambiguatedEntities(), which
allows the pagination path (ResponseInfo.nextPage() used by
OptionsTemplateHandler) to call getRelevantProperties() and dereference
explorerDB causing an NPE; add an override of getDisambiguatedEntities(String
uri, int page, int pageSize) in the disabled() anonymous subclass that returns
an empty/placeholder pagination-safe result (e.g., empty list or ResponseData
wrapper consistent with the method's contract) and ensure it does not access
explorerDB or call getRelevantProperties(), mirroring the safe-return pattern
used in getEntitiesByURIs(), getEntityInformation(), and getLabel.
| private String stripWikiepdiaContent(String text) { | ||
| int indexStart = text.indexOf("("), indexEnd; | ||
| if(indexStart > 0) { | ||
| if (indexStart > 0) { | ||
| indexEnd = text.indexOf(")", indexStart) + 2; | ||
| if(indexEnd != -1) { | ||
| if (indexEnd != -1) { | ||
| return text.replace(text.substring(indexStart, indexEnd), ""); | ||
| } |
There was a problem hiding this comment.
Prevent StringIndexOutOfBoundsException when “)” is missing or trailing.
indexEnd adds + 2 before checking, so -1 never survives, and indexEnd can exceed text.length(). This can crash on malformed abstracts.
🐛 Proposed fix
- int indexStart = text.indexOf("("), indexEnd;
+ int indexStart = text.indexOf("(");
+ int indexEnd;
if (indexStart > 0) {
- indexEnd = text.indexOf(")", indexStart) + 2;
- if (indexEnd != -1) {
- return text.replace(text.substring(indexStart, indexEnd), "");
- }
+ indexEnd = text.indexOf(")", indexStart);
+ if (indexEnd == -1) {
+ return text;
+ }
+ indexEnd = Math.min(text.length(), indexEnd + 2);
+ return text.replace(text.substring(indexStart, indexEnd), "");
} else if (indexStart == 0) {🤖 Prompt for AI Agents
In `@src/main/java/chatbot/lib/api/SPARQL.java` around lines 90 - 96, The method
stripWikiepdiaContent can throw StringIndexOutOfBounds because it adds +2 to the
closing-paren index before checking; change the logic in stripWikiepdiaContent
to (1) find indexStart with indexOf("(") and proceed only if indexStart >= 0,
(2) find the raw closing index via indexOf(")", indexStart) and verify it's not
-1, (3) compute indexEnd = rawClose + 2 but clamp it to text.length() (or
otherwise ensure indexEnd <= text.length()) before calling substring/replace,
and (4) only call text.substring(indexStart, indexEnd) when both indices are
within bounds; this prevents out-of-range accesses while preserving the intended
trimming behavior.
Improves error handling when CouchDB / Cloudant is unavailable by:
This makes the application more robust when the explorer database is down.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.