Skip to content

[fix] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present#18491

Open
Akanksha-kedia wants to merge 1 commit into
apache:masterfrom
Akanksha-kedia:fix-streamop-npe-compat-verifier
Open

[fix] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present#18491
Akanksha-kedia wants to merge 1 commit into
apache:masterfrom
Akanksha-kedia:fix-streamop-npe-compat-verifier

Conversation

@Akanksha-kedia
Copy link
Copy Markdown
Contributor

Description

Fix a NullPointerException in StreamOp.fetchExistingTotalDocs that occurs when the broker response contains a non-empty exceptions array.

Fixes #18490

Root Cause

The exceptions field in the broker query response is a JSON array ([{"errorCode": 410, ...}]), but the code was calling exceptions.get("errorCode") as if it were a JSON object. Jackson's ArrayNode.get(String) always returns null, so the subsequent errorCode.asInt() throws an NPE.

Changes Made

  1. Fix array access: Index into exceptions.get(0) first, then extract errorCode from the first exception object
  2. Add null guards: Defensively handle the case where the array element or errorCode field is missing
  3. Handle BROKER_SEGMENT_UNAVAILABLE (305): Treat this as a transient condition (segments still loading after a rolling upgrade) and return 0 to allow the caller's retry loop to continue, consistent with the existing BROKER_INSTANCE_MISSING handling

Testing Done

  • mvn spotless:check passes for pinot-compatibility-verifier
  • Code review — the fix is minimal and follows the existing pattern in the method

Checklist

…resent

The exceptions field in the broker query response is a JSON array, so
calling exceptions.get("errorCode") on an ArrayNode always returns null,
causing a NullPointerException. Fix by indexing into the first element.

Also treat BROKER_SEGMENT_UNAVAILABLE (305) as a transient condition
(segments still loading after rolling upgrade) and return 0 to allow the
caller's retry loop to continue, consistent with BROKER_INSTANCE_MISSING.
@Akanksha-kedia
Copy link
Copy Markdown
Contributor Author

@noob-se7en please review

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.70%. Comparing base (bd76ff0) to head (00c78d4).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18491      +/-   ##
============================================
+ Coverage     63.68%   63.70%   +0.01%     
  Complexity     1685     1685              
============================================
  Files          3266     3266              
  Lines        199821   199821              
  Branches      31022    31022              
============================================
+ Hits         127257   127294      +37     
+ Misses        62425    62394      -31     
+ Partials      10139    10133       -6     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 63.70% <ø> (+0.01%) ⬆️
temurin 63.70% <ø> (+0.01%) ⬆️
unittests 63.70% <ø> (+0.01%) ⬆️
unittests1 55.85% <ø> (+0.02%) ⬆️
unittests2 34.95% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NPE in StreamOp.fetchExistingTotalDocs when broker returns exceptions

2 participants