chadtmiller.com/knotless

Switch repository

Loading…

What changes

A failed write of a release said published already, whatever went wrong, with the real failure below it. It now says so only when the record is there, and otherwise gives the failure.

Why

Target::create took any failure to mean a lost race, since a PDS gives no error to tell. The reference PDS answers a create of a record that exists with a plain 500. So the record is read back to decide.

Part of Cargo registry: a stock PDS types a .crate's blob as application/gzip.

How it was checked

A test makes the same create three times against the PDS: with a bad token, then properly, then again. The rest is left to CI.

Shortcomings

A failure to read the record back reports that failure rather than the first.

🤖 Generated with Claude Code

This changes Target::create so a failed release write says published already only when the release record can be read back. Otherwise it returns the original failure. The reasoning holds: Target::open already reads the record the same way, get_record maps RecordNotFound to None, and the original error is kept when the record is absent. The new test covers a bad token, a first create, and a repeat create.

I only read the diff and the code around it. I did not run the tests or read any check results.

One thing for CI rather than review: the diff changes crates/knotless/src but adds no .changeset/ file, and the body has no Changeset: none line. The check check will fail on that. The change is visible to users, since it changes an error message, so a change file fits better than Changeset: none.

Carried forward: the change this pull makes at d422bbcf7b9c1c8e749ab6ddff6dd49df48d4bb2 is the one reviewed at 40a8716213d9020306498413cf0e9e0587c95ff8, so that review stands. What is new comes from its base.

This is still the same change. Target::create returns published already only when the release record can be read back. Otherwise it returns the original failure. The test covers a bad token, a first create and a repeat create. I read only the diff and did not run the tests.

My earlier review noted that the check workflow would fail because there is no .changeset/ file and no Changeset: none line in the body. The diff still has neither, so that is not addressed. It is something the check enforces, so I'm not withholding approval for it. The only check runs listed are cancelled ones for an older commit. Add a change file, since the error message is visible to users, or CI will fail it.

This is the same change as before. Target::create says published already only when the release record can be read back, and otherwise returns the original failure. The new commit adds the .changeset/release-published-already.md file I mentioned earlier, so that point is done. The test covers a refused token, a first create and a repeat create. I read only the diff and did not run the tests.

One small difference from the old code: the lost-race error is now a bare bail!(self.published()), where before the PDS failure sat below it as context. That is fine, because the record's presence is what the message reports. Keep the failure as context if you want it in logs.