[dmccoystephenson]

Summary

• report(String, Double, Map) could throw into the host program, against the "never throws" promise. The caller's tags map was copied by withVersion and serialized by json() on the caller's thread with no guard, so a map being changed by another thread escaped as ConcurrentModificationException, and a raw map holding a non-String escaped as ClassCastException.
• That read is now wrapped in catch (RuntimeException ...), and the one report is dropped. The client keeps working afterwards.
• The drop is not logged. report() runs on the host's thread, where a Handler may do IO, and the two existing drops on that path (a full queue, too many counted events) are silent for the same reason.
• The README "What report promises" table, the class Javadoc, the report Javadoc and the Builder.logger Javadoc now say so.

Closes #37

Merge gate

The do-not-auto-merge list was checked against this diff: the wire format, REASON_*/ENV_*, the opt-out precedence, QUEUE_CAPACITY, the timeouts, SERVER_WIDE_CONFIG_CONTENT, pom.xml, the version strings and the README "Opting out" and "Keys" sections are all untouched. A report with readable tags produces the same bytes as before.

Deferred issues

• withVersion pushes an event already at MAX_TAGS over the server's tag limit #13, A quoted enabled: "false" in the server-wide config does not turn reporting off #15, A comment-only enabled: line in the server-wide config shadows a later enabled: false #27, A coalesced count at MAX_TAGS, or on an event with its own coalesced tag, is sent without its count tag #31: each changes the wire format or the opt-out contract and is waiting on a maintainer decision, so none was picked up this cycle.

Test plan

• report_aTagsMapChangedMidReadIsDroppedWithoutThrowing and report_aNonStringTagSmuggledInByARawMapIsDroppedWithoutThrowing fail with TraceClient.java stashed and pass with it restored.
• mvn -B verify (Java 21): 99 tests run, 0 failures (97 before).
• CI matrix (Java 8, 17, 21) green on the PR head.

CI does not run the client inside a real Spigot server. The fix depends on no host behaviour, so no manual smoke test is called for.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric (scored against the diff and CI run 41829012617, head 5248039):

• Scope: PASS. Three files changed (README.md, TraceClient.java, TraceClientTest.java), all for report() throws when the tags map fails while being read #37. There is no formatting churn. The Builder.logger Javadoc reflow comes from the sentence that was changed.
• Tests-new: not applicable. No public members were added.
• Tests-fix: PASS. With TraceClient.java stashed, both new tests failed (ConcurrentModificationException at TraceClientTest:248, ClassCastException at TraceClientTest:272; 2 run, 2 failures). With it restored, the full suite passed.
• Sibling structure: PASS. The names follow the report_* series, with Arrange/Act/Assert comments, the shared loopback server, and RecordingHandler on a uniquely named logger.
• Sibling renames: not applicable.
• Docs: PASS. The README "What report promises" row, the class Javadoc, the report Javadoc and the Builder.logger Javadoc all say the same thing. The other Phase 7 rows describe nothing this diff changes.
• Issue resolution: PASS. report() throws when the tags map fails while being read #37 names report, and both of its reproductions are the two new tests.
• CI: PASS. build (8), build (17) and build (21) are green on the head. Locally, mvn -B verify (Java 21) ran 99 tests with 0 failures (97 before); the surefire report lists 99 test cases.
• No dependencies: PASS. pom.xml is untouched, and TraceClient.java imports only java.*.
• Single file: PASS. git ls-files src/main lists only TraceClient.java.
• Java 8: PASS. The build (8) leg is green, and maven.compiler.release is unchanged. The test code uses only Java 8 APIs.
• Never throws: PASS. Everything that reads the caller's map now sits inside the catch. What runs later under lock uses merged, a private copy whose keys and values the first json() call has already cast to String, so new TreeMap<>(merged) cannot fail on them.
• Non-blocking and bounded: PASS. No collection was added, and nothing new runs on the caller's thread. The drop is deliberately unlogged for that reason.
• Environment seam: PASS. System.getenv appears only in the environment initializer and the comment above it.
• Version agreement / opt-out contract: not applicable. Neither was changed.

Observations:

• TraceClient.java report (judgment call for the maintainer): the silent drop is the right fit for a transient concurrent change, but it is less friendly to a programming error. A raw map that always holds an Integer drops every report of that event, and nothing says why, even at FINE. A FINE line on the caller's thread would conflict with the no-IO rule for report(), and handing it to the sender thread would need its own bound. The silent drop was kept as the smallest change, and it is documented in all four places. It costs nothing to revisit.
• TraceClientTest.java report_aTagsMapChangedMidReadIsDroppedWithoutThrowing: the map throws from entrySet() on every call, which stands in for the race deterministically. It covers the path where the failure happens, not the race itself.
• Outside this diff: build() can still throw if environment.apply throws. The fix needs an opt-out decision, so it is tracked separately in build() throws if reading the environment fails #39.

Summary: no blocking findings. The fix is proven by a recorded fail-then-pass, CI is green on all three JDKs, and no do-not-auto-merge path matches this diff.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson