fix: report a missing refresh token as AuthException

[mlwelles]

Stacked on #294 ("fix: make DgraphAsyncClient non-blocking to avoid ForkJoinPool.commonPool starvation") as GitHub stack #298. The base is that branch, not main, because #294 rewrote the exact lines this changes. Read #294 first; the diff here is just this commit.

Merge path

Draft until the test suite has actually run on it. ci-dgraph4j-tests.yml and trunk.yml both gate their pull_request trigger on branches: main, so while this PR targets a feature branch it gets CodeQL but neither the test suite nor the linters. GitHub still reports it MERGEABLE and CLEAN, because absent checks cannot fail — judge it by which checks are present, not by its merge state.

Merging bottom-up closes that gap with no changes to CI:

1. Merge fix: make DgraphAsyncClient non-blocking to avoid ForkJoinPool.commonPool starvation #294. It has a full green run against main.
2. This PR is retargeted to main automatically. The repo has delete_branch_on_merge enabled, and GitHub retargets open PRs based on a deleted head branch to the merged PR's base.
3. Mark this PR ready for review. ready_for_review is in the workflow's trigger list, so with the base now main the full suite and the linters run here.
4. Merge once green.

gh stack merge 298 would instead land both PRs atomically, which is faster but merges this one without the suite ever running on it — the post-merge push: main run would catch a problem after the fact rather than before. #299 ("ci: run tests and linters on pull requests to any base branch") removes the base-branch filter that causes this, and once it merges the checks appear here and either path is validated. Until then, prefer the sequence above.

Description

retryLogin() signalled "no refresh JWT available" with a bare java.lang.Exception. Exceptions.translate has no case for that, so it fell through to the generic wrapper and the caller saw a DgraphException reporting INTERNAL — an authentication failure presented as an internal error. It is the one gap left in the typed exception hierarchy added in 25.0.0, which already defines AuthException for exactly this condition.

This fails with AuthException carrying Status.UNAUTHENTICATED instead. translate passes DgraphException subclasses through untouched, so it reaches the caller intact.

Compatibility

• Retryability is unchanged. AuthException inherits isRetryable() == false, exactly as the generic DgraphException did, so withRetry behaves identically.
• catch (DgraphException) and catch (StatusRuntimeException) are unaffected — AuthException is a subclass of both.
• Only code that distinguishes AuthException sees a difference, which is the point of the change.
• The status code moves from INTERNAL to UNAUTHENTICATED. Code switching on the gRPC status code of this specific failure would observe that.

Tests

RetryLoginTest covers two cases, neither needing a server, since the missing-token check returns before any RPC is issued:

Test	What it pins
refreshWithoutATokenFailsWithAuthException	the direct call fails with AuthException and UNAUTHENTICATED
expiredTokenWithNoRefreshSurfacesAuthException	the path a user hits — an expired token with no way to refresh it — surfaces AuthException through runWithRetries

Both fail before this change: the first sees a raw java.lang.Exception, the second sees DgraphException: INTERNAL.

Checklist

• Code compiles correctly and linting passes locally
• For all code changes, an entry added to the CHANGELOG.md file describing and linking to
  this PR
• Tests added for new functionality, or regression tests for bug fixes added as applicable

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

• Bug Fixes

  • JWT refresh attempts without an available refresh token now fail with a clear UNAUTHENTICATED authentication error instead of a generic internal error.
  • Expired-token refresh failures now preserve the appropriate authentication error details.

• Tests

  • Added coverage for missing refresh tokens and expired-token retry behavior.

[mlwelles]

No CI ran on this PR, and the green state is misleading. Marking it draft until that is fixed.

Both workflows are gated on the base branch:

• ci-dgraph4j-tests.yml: pull_request: branches: [main]
• trunk.yml: pull_request: branches: main

This PR targets fix/async-client-non-blocking-retries, so neither triggered. Nothing ran — not the test suite, not the linters. GitHub still reports mergeable: MERGEABLE and mergeStateStatus: CLEAN, because with no checks present there are none to fail. Do not read that as validated.

What I verified locally instead

• ./gradlew compileJava compileTestJava succeeds.
• RetryLoginTest passes across three runs, and both of its assertions fail when the change is reverted (one sees a raw java.lang.Exception, the other DgraphException: INTERNAL).
• trunk fmt and trunk check are clean on all three changed files.
• The cluster-dependent tests could not run: they need a dgraph/dgraph:local image built from Dgraph source, and I have no Docker daemon available. This change touches one exception type in retryLogin() and no transaction or query path, and fix: make DgraphAsyncClient non-blocking to avoid ForkJoinPool.commonPool starvation #294 ("fix: make DgraphAsyncClient non-blocking to avoid ForkJoinPool.commonPool starvation") already has a green full-suite run on the parent commit.

To land this

Merge #294 first, then retarget this PR to main. The commit applies cleanly on top of merged-#294 code, and CI will run on the retargeted PR. Please do not merge it while it targets a feature branch.

Separately: widening those two triggers (for example to branches: [main, 'fix/**']) would let stacked PRs be validated at all. That is a repo policy change with a real cost — every stacked PR would build Dgraph from source — so I have not made it here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 00454ce7-0fe9-4509-a4b9-8354eef5ef57

📥 Commits

Reviewing files that changed from the base of the PR and between d8d45b5 and e6cdadd.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/main/java/io/dgraph/DgraphAsyncClient.java
• src/test/java/io/dgraph/RetryLoginTest.java

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The refresh path now returns AuthException with UNAUTHENTICATED when no refresh JWT is available. New tests verify direct failure and propagation through retry handling. The changelog records the correction.

Changes

JWT refresh error classification

Layer / File(s)	Summary
Refresh error contract src/main/java/io/dgraph/DgraphAsyncClient.java, CHANGELOG.md	retryLogin now creates an AuthException with UNAUTHENTICATED when the refresh JWT is missing. The changelog records the corrected error classification.
Refresh error validation src/test/java/io/dgraph/RetryLoginTest.java	Tests verify the missing-token failure and confirm that CompletableFutures.runWithRetries preserves the AuthException.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to e6cda

Missing refresh tokens now produce an unauthenticated authentication error instead of a generic internal error, while retry propagation remains intact. The covered checks pass and no concrete merge-blocking risk is evidenced.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: reporting a missing refresh token as AuthException.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/typed-auth-error-for-missing-refresh-token

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}