OSIS-156: log empty bucket list at DEBUG, not as a failure

[anurag4DSB]

Intent: why does this change exist?

When a tenant legitimately has no buckets, the platform answers with a 404/error and OSIS recovers to an empty list. That expected case was still logged as a failure (WARN after OSIS-163, ERROR with a full stack before it), which reads as a problem to operators when nothing is wrong.

System impact: what's affected, including downstream?

One catch block in getBucketList in osis-core. Only the log level and message change; the returned response and HTTP behavior are identical, so OSE sees no difference.

Preserved behavior: what explicitly stays the same?

getBucketList still returns an empty PageOfOsisBucketMeta with the same pageInfo on the recovery path. The happy path is untouched. A genuine fault still surfaces through the returned empty page exactly as before.

Intended change: what's different after this PR?

The empty-bucket recovery is logged at DEBUG with a concise message instead of WARN, and continues to carry no stack trace. Adds a ListAppender regression test asserting the empty case returns an empty page and is never logged at ERROR or WARN nor dumps a stack.

Verification: how do we know this worked, or how would we know if it didn't?

Ran ./gradlew :osis-core:test jacocoTestCoverageVerification: all tests pass and coverage stays above the 75% gate. The new test fails if the recovery log returns to WARN/ERROR or starts carrying a stack trace.

[claude]

• ScalityOsisServiceImpl.java:1099 — The catch block demotes all exceptions to DEBUG, not just the expected 404. Vault/network/auth errors will also be silently hidden from operators. Suggest narrowing DEBUG to S3ServiceException with NOT_FOUND status and keeping WARN for everything else. The test should be updated accordingly to cover the genuine-failure path.
  
  Review by Claude Code

[claude]

Blocking issue — wrong exception type makes the fix a no-op in production.

• ScalityOsisServiceImpl.java:1096-1097 — The catch block checks e instanceof S3ServiceException, but s3Client.listBuckets() is called on a raw AWS SDK AmazonS3 client (constructed by S3Impl.getS3Client()), which throws AmazonS3Exception on failure. S3ServiceException (Spring ResponseStatusException) and AmazonS3Exception (AWS AmazonServiceException) are unrelated hierarchies, so the instanceof check never matches. In production the 404 still logs at WARN — the old behavior.

• ScalityOsisServiceMiscTests.java:192-195, 245-248 — Both new tests mock listBuckets() to throw S3ServiceException, matching the (incorrect) production code. The tests pass but validate a path that doesn't execute in the real system. Should throw AmazonS3Exception with setStatusCode(404) to match what the AWS SDK actually produces.

Fix: change the instanceof check to AmazonS3Exception and compare getStatusCode() == 404. Update both test mocks to throw AmazonS3Exception. The pre-existing testGetBucketListErr has the same mock-type issue and should be updated as well for consistency.

Review by Claude Code

[anurag4DSB]

Addressed in 2dd1765: narrowed the DEBUG path to the expected case (S3ServiceException with NOT_FOUND status) and kept WARN for genuine Vault/S3/network/auth faults, so real failures stay visible to operators while the empty-page contract is unchanged. Added testGetBucketListGenuineFaultLogsAtWarn to cover the genuine-fault path; the existing empty-list test still asserts DEBUG-only. Thanks for the catch.

[claude]

LGTM — well-scoped change. The logic correctly distinguishes a 404 (expected empty-bucket case) from other faults, the empty-page contract is preserved on both paths, and the two new tests cover both branches with log-level assertions. One minor note left inline about getStatus() deprecation for the eventual Boot 3 migration.

Review by Claude Code

[claude]

LGTM — clean, well-scoped change. The exception split is correct: tenantSession.run() passes exceptions through unwrapped, and getS3Client() returns the raw AWS AmazonS3 interface, so the instanceof AmazonS3Exception check in the catch block will match the actual exception thrown by listBuckets(). The removed S3ServiceException import is confirmed unused elsewhere in the test file. Both new tests (404→DEBUG, 500→WARN) cover the two branches of the conditional, and the ListAppender cleanup in finally is properly handled.

Review by Claude Code

[anurag4DSB]

Superseded by #167, which re-creates this change minimally on top of main (and keeps genuine bucket-list faults at ERROR with their trace; only the expected empty case is quieted). Closing this one.