Skip to content

Keep build() from throwing on an unconvertible plugins path - #8

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/build-never-throws-on-invalid-path
Oct 2, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
feature/build-never-throws-on-invalid-path

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

  • readServerWideConfig called File.toPath() outside its try. A plugins path the file system cannot represent (e.g. a NUL in the name) therefore let InvalidPathException escape build(), which is documented as "Never throws". The conversion has been moved inside the existing catch (IOException | RuntimeException), so such a path is now logged at FINE and treated as enabled, like every other server-wide IO failure.
  • A regression test has been added: serverWideConfig_aPathThatCannotBeConvertedIsLoggedFineAndTreatedAsEnabled.
  • The README, the Javadoc, the wire format, the opt-out constants and the version strings are all unchanged. The documented behaviour ("an IO failure is logged at FINE and counts as enabled") already describes the fixed behaviour.

Closes #7

Test plan

  • mvn -B verify run locally on Java 21: 47 tests, 0 failures
  • The new test was confirmed to FAIL before the fix (InvalidPathException escaped build()) and to PASS after it
  • CI matrix (Java 8, 17, 21) green on the PR head

Triage notes

No other issues were open at triage, so none were deferred. No version bump was made; that is left to the maintainer.

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

File.toPath() ran outside the try in readServerWideConfig, so a path
the file system cannot represent (a NUL in the name) let an
InvalidPathException escape build(), which promises never to throw.
It is now converted inside the try and treated like any other IO
failure: logged at FINE, counted as enabled.

Closes #7

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric (anchored on CI run 36979371739, where build (8), build (17) and build (21) all passed on the PR head):

  • Scope: PASS. Only TraceClient.java (the readServerWideConfig fix) and TraceClientTest.java (its regression test) are modified.
  • Tests-new: PASS. No new public members were added.
  • Tests-fix: PASS. The new test was run locally with the fix absent and FAILED (InvalidPathException escaped build()). With the fix applied it PASSED, and the full suite ran 47 tests with 0 failures.
  • Sibling structure: PASS. No new files. The test follows the neighbouring serverWideConfig_ioFailureIsLoggedFineAndTreatedAsEnabled pattern (Arrange/Act/Assert, RecordingHandler, a uniquely named logger).
  • Sibling renames: PASS. Nothing was renamed. The local file was split into location (File) and file (Path) inside one method.
  • Docs: PASS. The README "Opting out" row ("An IO failure is logged at FINE and counts as enabled") and the build() Javadoc ("Never throws") already describe the fixed behaviour, so no doc change is needed.
  • Issue resolution: PASS. build() throws InvalidPathException when the plugins directory path cannot be converted #7 names readServerWideConfig's out-of-try toPath(), which is exactly what changed.
  • CI: PASS. All three matrix legs are green.
  • 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. build (8) is green, which also confirms that Java 8's UnixPath rejects the NUL, so the test is meaningful on the floor JDK.
  • Never throws: PASS. The only code left outside the try is new File(File, String), which throws only on a null child, and SERVER_WIDE_CONFIG_PATH is a constant.
  • Non-blocking & bounded: PASS. report() is untouched.
  • Environment seam: PASS. System.getenv appears only in the seam comment and initializer.
  • Version agreement / opt-out contract: PASS (not applicable). No version string, REASON_*, ENV_* or precedence change.

Observations:

  • src/main/java/software/stephenson/trace/TraceClient.java:301: the FINE message now prints the File rather than the Path. Both give the same string for any representable path. For the unrepresentable case, the raw path (including the NUL) is printed, which is acceptable at FINE.
  • The trigger is contrived for a real getDataFolder().getParentFile(), so this is contract hardening rather than a reported field failure. No version bump was made.

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


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 94fa0db into main Oct 2, 2026
3 checks passed
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.

build() throws InvalidPathException when the plugins directory path cannot be converted

1 participant