OSIS-155: pin headTenant pre-create 404 to INFO, no stack trace#163
Conversation
OSIS-163 already removed the service-layer logger.error that dumped a full stack on the expected pre-create 404 during tenant activation, and routed that NotFoundException through the error boundary, which logs 4xx once at INFO with no trace. This adds regression tests so the misleading 'invalid account ID' stack trace cannot come back. No production change: a service-layer test asserts the pre-create 404 still returns NotFoundException and is never logged at ERROR/WARN with a stack trace, and a boundary test pins that the exact headTenant 404 is logged once at INFO without a trace.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
|
Addressed in e783151: removed the redundant boundary test. It exercised the same respond() path as testNotFoundMapsTo404LoggedOnceAtInfoWithoutTrace (only the message string differed, and respond() does not branch on it), so it added no code-path coverage. The service-layer test in ScalityOsisServiceTenantTests remains the regression pin. |
|
LGTM — clean, well-structured regression test. One nit on the PR description: it says "Two regression tests" but the final diff only adds one (the service-layer test). The boundary test was added in commit 1 then dropped as redundant in commit 2 — the description still references both. Worth a quick edit for posterity since the PR body becomes the merge record. Review by Claude Code |
|
Superseded by #166, which re-creates this change minimally on top of main instead of stacked on the closed refactor chain. Closing this one. |
1 participant
Intent: why does this change exist?
During tenant activation OSE calls headTenant before the account exists, so the platform answers 404 and OSIS used to log a misleading "invalid account ID" error with a full stack trace even though the create that follows succeeds. The error-boundary work already fixed the behavior; this pins it so it cannot regress.
System impact: what's affected, including downstream?
Test-only change: osis-core adds a regression test; osis-app drops a redundant near-duplicate test. No production code is touched, so no runtime behavior and no downstream OSE or Vault interaction changes.
Preserved behavior: what explicitly stays the same?
headTenant still returns a 404 (NotFoundException) for the pre-create case, and the error boundary still translates and logs every 4xx exactly once at INFO. The activation flow is unchanged.
Intended change: what's different after this PR?
A service-layer regression test (Logback ListAppender) asserting the pre-create 404 is never logged at ERROR or WARN and carries no stack trace. A near-duplicate boundary test was dropped per review, since it exercised the same respond() path as the existing testNotFoundMapsTo404LoggedOnceAtInfoWithoutTrace.
Verification: how do we know this worked, or how would we know if it didn't?
Ran
./gradlew :osis-core:test :osis-app:test jacocoTestCoverageVerification: all tests pass and coverage stays above the 75% gate. The new test would fail if anything reintroduced an ERROR-level or stack-trace log for the expected pre-create 404. The underlying behavior was already fixed by the error-boundary change, so no further production change is needed beyond this regression test.