Skip to content

dataprep: fix err order and %d/string in ListPiecesHandler#673

Open
parkan wants to merge 3 commits intomainfrom
fix/piece-list-err-handling
Open

dataprep: fix err order and %d/string in ListPiecesHandler#673
parkan wants to merge 3 commits intomainfrom
fix/piece-list-err-handling

Conversation

@parkan
Copy link
Copy Markdown
Collaborator

@parkan parkan commented Apr 23, 2026

Summary

Two small bugs in `ListPiecesHandler`, both in the same block:

  1. The `Find(&sourceAttachments)` error was checked after the `len == 0` guard. A DB failure (connection lost, timeout) yielded an empty slice which the handler then reported as 404 "preparation not found" -- masking 500s as missing data.

  2. The not-found format string used `%d` against the `id string` argument, producing messages like `preparation %!d(string=foo) not found`.

Fix: check err first; format the id with `%s`.

Test plan

  • `go test ./handler/dataprep/...`
  • CI

parkan added a commit that referenced this pull request Apr 23, 2026
## Summary
PR #669 added \`//go:generate make -C sol bytecode\` to
\`util/testutil/fvm_precompiles.go\`, which calls \`forge build\` +
\`forge inspect\`. The devcontainer's Dockerfile only copied \`anvil\`
from the Foundry image, not \`forge\`.

Effect: CI's \"Generate swagger code\" step runs \`go generate ./...\`
whenever codegen-gated paths (\`api/\`, \`handler/\`, \`cmd/\`,
\`storagesystem/\`, \`docs/gen/\`, \`model/\`, \`singularity.go\`,
\`docgen.sh\`) change. On those PRs, \`make -C sol bytecode\` fails with
\`forge: No such file or directory\`.

Currently blocks #670, #673, #674.

## Test plan
- [ ] CI devcontainer build includes \`/usr/local/bin/forge\`
- [ ] \`go generate ./...\` succeeds inside the devcontainer
@parkan parkan force-pushed the fix/piece-list-err-handling branch from c3f21f0 to 057023f Compare April 23, 2026 15:56
parkan added a commit that referenced this pull request Apr 23, 2026
## Summary
\`forge inspect\` appends a CBOR metadata section to the
deployedBytecode: an IPFS hash of the compiler metadata JSON plus the
solc version tag. That IPFS hash drifts between forge versions even with
\`solc\` pinned in \`foundry.toml\`. Locally (forge 1.5.1) it matches
what was committed in #669; in CI (\`foundry:latest\`, newer) \`go
generate ./...\` produces different trailing bytes and \`git diff
--exit-code\` fails.

Fix: \`bytecode_hash = \"none\"\` + \`cbor_metadata = false\` -- solc
omits the metadata section entirely. Output is deterministic across
forge versions and solc patches. Regenerated the three .txt files
against the new config; runtime bytecode is unchanged (FVM precompile
mocks don't consult the metadata).

Currently blocks #670, #673, #674 after #675 unblocked the
missing-\`forge\` issue.

## Test plan
- [x] \`make -C util/testutil/sol clean bytecode\` produces the
committed output under forge 1.5.1
- [x] \`go test ./util/testutil/...\` pass
- [x] \`go test ./service/dealpusher/...\` pass (DDO / PDP mocks
exercise the stripped bytecode)
- [ ] CI
Two issues in ListPiecesHandler:

1. The error from Find(&sourceAttachments) was checked after the
   len==0 guard, so a real DB error (connection lost, timeout) was
   mapped to a 404 'preparation not found' instead of propagating.

2. The 'not found' format string used %d against the string handler
   argument id, producing 'preparation %!d(string=foo) not found'.
@parkan parkan force-pushed the fix/piece-list-err-handling branch from 057023f to d8881ec Compare April 23, 2026 17:32
@parkan parkan requested a review from anjor April 23, 2026 19:06
@parkan parkan enabled auto-merge (squash) April 24, 2026 14:02
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.

2 participants