[dmccoystephenson]

Summary

• A truthy non-string base_url, application, version, key or install_id raised an AttributeError from .strip() that did not say which argument was wrong. A ValueError is now raised instead, such as version must be a string, not float, so the rule that constructor errors name their argument holds for these values too.
• The message names the type and never the value, so a rejected key is not repeated in whatever logs the error.
• Falsy values such as key=0 never reached .strip() and keep their meaning, and a test pins that.
• One input that used to construct is now rejected: bytes (and any other truthy non-string with a .strip()). bytes for application, version, key or install_id built a disabled client without complaint, and key=b"k" built an enabled one, whose reports could not be sent because the Authorization header is built by adding it to a str. Such a program now stops at startup with a named error, including when reporting is off. That is accepted here because the same value would lose every report once reporting was on, and it matches the fix A non-string constructor argument raises an AttributeError that does not name the argument #33 suggests.
• test_a_dropped_report_is_logged_at_debug_with_its_name is made deterministic. It failed in 2 of 7 full-suite runs on unmodified main, so it could have failed this PR at random. The wait is the one test_close_on_a_full_queue_drops_the_oldest_report_and_still_stops_the_sender already uses.
• The README and the constructor docstring state the new error.

Closes #33
Closes #34

Test plan

• python3 -m unittest on Python 3.8.10: 72 tests, OK, in 6 consecutive full-suite runs (69 tests before this change)
• With the fix in trace_client/trace_client.py stashed, test_a_value_that_is_not_a_string_is_rejected_by_the_argument_it_was_given_for and test_a_rejected_key_is_not_repeated_in_the_error errored with AttributeError: 'int' object has no attribute 'strip'; with it restored, both passed
• test_an_empty_value_of_another_type_keeps_its_old_meaning passed with the fix stashed and with it restored, as it characterizes behaviour that must not change
• Forced race for test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34: with the sender held until just before overflow-event, the original test failed with 1 != 0 : a drop names the event it dropped: ['[trace] queue full, dropped filler'], and the rewritten test passed. The harness was a scratch file and is not committed
• test (3.8), test (3.10) and test (3.12) green on the PR head (70caea5, run 42338761845)

Trace server and Java

No Java-visible behavior changed: nothing on the wire changes, and neither the opt-out values nor the disabled_reason strings are touched. This gap comes from Python's dynamic typing, so a counterpart in trace-client-java is not expected, but that source could not be read from this session and the point is unverified.

Issues not picked

• A 301, 302 or 303 answer sends the reporting key to the redirect target as an empty GET #31: changes redirect behaviour in _send, which is wire format held for owner review, and it deserves its own PR.
• The version tag is added even when it pushes an event past MAX_TAGS #16: still blocked on the owner decision and the trace-client-java lockstep recorded on that issue.

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 (head 70caea5). This review was performed in the same session that wrote the change, so it is not an independent review.

• Scope: PASS — three files changed. README.md, trace_client/trace_client.py and the three new tests serve A non-string constructor argument raises an AttributeError that does not name the argument #33; the rewritten test serves test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34, which was filed this cycle after the flake appeared on unmodified main.
• Tests-new: PASS — no public name added; the new check is exercised through the constructor by three new tests.
• Tests-fix: PASS — for A non-string constructor argument raises an AttributeError that does not name the argument #33, with trace_client/trace_client.py stashed, two new tests errored with AttributeError: 'int' object has no attribute 'strip', and both passed once restored. For test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34, a forced interleaving failed the original test with the message seen in the wild, and the rewritten test passed.
• Sibling structure: PASS — no new files; the new tests sit beside test_constructor_errors_name_the_argument_they_reject.
• Sibling renames: PASS — nothing renamed.
• Docs: PASS — the README's version section and the constructor docstring state the error. The wire format, opt-out and promises sections are unaffected.
• Issue resolution: PASS — all five arguments named in A non-string constructor argument raises an AttributeError that does not name the argument #33 are checked and the named test in test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34 is changed.
• CI: PASS — test (3.8), test (3.10) and test (3.12) passed on run 42338761845.
• Stdlib-only: PASS — no import added; pyproject.toml untouched.
• Py3.8 floor: PASS — test (3.8) green on the PR head.
• Never-raises / never-blocks: PASS — the check runs only in the constructor, the one path allowed to raise; report(), _drain(), _send() and close() are unchanged.
• Log level: PASS — no _LOG call added.
• Version agreement: PASS — 0.4.2 everywhere; no version string changed.
• Exports: PASS — no public name added.
• Java parity noted: PASS, with a caveat — the PR body states no Java-visible behavior changed, and that trace-client-java could not be read from this session.

Finding, corrected before posting: the first PR description said only values that already stopped the constructor change, and that was wrong. bytes, and any other truthy non-string with a .strip(), used to construct and is now rejected, including on a disabled client. The description was corrected to say so. The change is kept, since such a value loses every report once reporting is on.

Not changed: a falsy non-string such as key=0 still means no key, so an all-digit key of 0 in a settings file is silently a disabled client. That is existing behaviour, left alone so that no working program starts to stop at startup.

trace_client/trace_client.py:147 — the check precedes every .strip() and the opt-outs, so it also fires under DO_NOT_TRACK=1, like the existing blank and overlong checks.
tests/test_trace_client.py:478 — this new line is 116 columns, in a file that already has many lines past 110; left as is.

No do-not-auto-merge path matches: pyproject.toml, .github/workflows/, LICENSE, the version strings, _send, _json, the opt-out values and the disabled_reason strings are all untouched, and no file loses more than 50 lines.

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

---

drafted by Claude on behalf of Daniel Stephenson