Skip to content

Fix the findings of the 2026-09-07T19-09 code analysis - #15

Merged
dirkjink merged 2 commits into
mainfrom
fix/code-analysis-2026-09-07
Sep 7, 2026
Merged

Fix the findings of the 2026-09-07T19-09 code analysis#15
dirkjink merged 2 commits into
mainfrom
fix/code-analysis-2026-09-07

Conversation

@dirkjink

@dirkjink dirkjink commented Sep 7, 2026

Copy link
Copy Markdown
Member

Remediation of docs/assessment/CODE_ANALYSIS-2026-09-07T19-09-00.md (0 Critical, 1 High, 4 Medium, 8 Low, 3 systemic patterns), per the review workflow: fix commit 1c8b048 first, then the report with per-finding status lines.

Fixed

  • High (test): the appender-level exactly-once test now observes the late send failure through captured meters and joins the send worker, so a broken claim wiring turns it red (mutation-checked).
  • Medium: a send the Kafka client fails through its synchronous callback (metadata timeout, buffer exhausted, record too large) counts as fallback only, never as dispatched (SendCallback with errorReported; new producer double + tests, mutation-checked).
  • Medium: FallbackDispatcher.close() lets the worker drain for the whole shutdown budget before interrupting (was effectively 200 ms).
  • Medium: restart symmetry - stop() keeps the fallback appender attached, start() restarts it and re-arms the one-shot error report.
  • Medium: the Kafka callback retains a detached DiversionClaim instead of the whole PendingSend (payload copy); README documents the heap bound under a slow broker.
  • Low: metrics unbound after the teardown they should observe; bind/unbind serialized; idempotence validation covers acks; fallback worker carries the reentry guard; reconfiguration rebind gap documented; assertion-less and >= tests replaced; test fixtures close what they start.

Deliberately left

  • Finding 9 resolved by documentation (Logback offers no post-reconfiguration hook).
  • Patterns P2 (bounded wall-clock polling) tolerated, P3 (injectable shutdown time source) deferred - rationale in the report.

mvn verify (offline, incl. ktlint): 264 tests green.

🤖 Generated with Claude Code

https://claude.ai/code/session_015GfdGp7eUrjJKBvcJUx3q2

dirkjink and others added 2 commits September 7, 2026 19:30
- H: the appender-level exactly-once test observes the late send
  failure through captured meters and joins the send worker, so a
  broken claim wiring now turns it red (mutation-checked)
- M: a send the Kafka client fails through its synchronous callback
  (metadata timeout, buffer exhausted, record too large) counts as
  fallback only, never as dispatched; SendCallback replaces the
  lambda; producer double + tests for the ApiException path
- M: FallbackDispatcher.close() lets the worker drain for the whole
  shutdown budget before interrupting (was 200 ms), mirroring
  SendDispatcher; INTERRUPT_GRACE_MS replaces GRACEFUL_DRAIN_WAIT_MS
- M: restart symmetry - stop() stops the fallback appender without
  detaching it, start() restarts it and re-arms the one-shot error
  report; restart test
- M: the Kafka callback retains a detached DiversionClaim instead of
  the whole PendingSend (payload copy); README/KDoc document the
  heap bound under a slow broker (buffer.memory in event size)
- L: metrics are unbound after the dispatcher closes, so shutdown
  diversions/drops are scraped during the teardown
- L: bindMeterRegistry and the unbind in stop() are serialized by a
  ReentrantLock with the started state re-checked under it
- L: idempotence validation covers acks=all; an explicit
  enable.idempotence=true suppresses the class's acks=1 default
- L: the fallback worker carries the reentry guard; addAppender KDoc
  states the no-SLF4J-logging contract for fallback appenders
- L: Logback-reconfiguration rebind gap documented (KDoc, README)
  with the public bindAppenders() as the manual path
- L: assertion-less tests now assert accounting; exact counts in the
  FallbackDispatcher drop/double-close tests
- P1: appenders, sender contexts and fallback dispatchers built by
  test fixtures are closed after each test

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GfdGp7eUrjJKBvcJUx3q2
Report of the defect analysis at 6c4f2b8 (0 Critical, 1 High, 4 Medium,
8 Low, 3 systemic patterns); every finding carries its status line
referencing the fix commit 1c8b048 - 12 fixed, finding 9 resolved by
documentation, patterns P2/P3 deliberately left with rationale.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GfdGp7eUrjJKBvcJUx3q2
@dirkjink
dirkjink merged commit ec49b24 into main Sep 7, 2026
6 checks passed
@dirkjink
dirkjink deleted the fix/code-analysis-2026-09-07 branch September 7, 2026 17:39
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.

1 participant