Skip to content

Commit c206b6e

Browse files
0g-peterzhbclaude
andcommitted
transfer: name the file in the existence errors (#200)
checkFileExistence reported both outcomes without saying which file it meant, so what reached the user was "Failed to download file: Failed to check file existence: File already exists" with no path at all. For the caller's own destination that is merely terse. For a fragment temp file it misleads: fragments are downloaded to <root>.temp in the process working directory, and if one survives an attempt - the copy into the output failed, os.Remove failed, the process was killed mid-iteration - every later attempt reports "File already exists" for a file the caller never created. They inspect the output file they asked for, find nothing wrong, delete it, retry, and get the same error, with nothing indicating what is actually blocking them. Include the path in both. ErrFileAlreadyExists is wrapped rather than replaced so the errors.Is checks that depend on it keep matching - download_dir relies on it in three places, and the fragment loops in #199 will too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent dfec67f commit c206b6e

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

transfer/downloader.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,16 @@ func checkFileExistence(filename string, hash common.Hash) error {
288288
return errors.WithMessage(err, "Failed to create file merkle tree")
289289
}
290290

291+
// Name the file. Both of these are reported for a fragment's temp file as well as for
292+
// the caller's own destination, and without the path the message sends the reader to
293+
// the wrong place: they see "File already exists", inspect the output file they asked
294+
// for, find nothing wrong with it, and never learn the obstacle is a <root>.temp they
295+
// did not create.
291296
if tree.Root() == hash {
292-
return ErrFileAlreadyExists
297+
return errors.WithMessagef(ErrFileAlreadyExists, "%v", filename)
293298
}
294299

295-
return errors.New("File already exists with different hash")
300+
return errors.Errorf("File already exists with different hash: %v", filename)
296301
}
297302

298303
func (downloader *Downloader) downloadFile(ctx context.Context, filename string, root common.Hash, info *node.FileInfo, withProof bool) error {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package transfer
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
8+
"github.com/0gfoundation/0g-storage-client/core"
9+
"github.com/ethereum/go-ethereum/common"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// Both existence errors are reported for a fragment's temp file as well as for the
15+
// caller's own destination. Without the path, someone hitting the fragment case reads
16+
// "File already exists", inspects the output file they asked for, finds nothing wrong
17+
// with it, and never learns the obstacle is a <root>.temp they never created.
18+
func TestCheckFileExistence_NamesTheFile(t *testing.T) {
19+
path := filepath.Join(t.TempDir(), "0x1111.temp")
20+
require.NoError(t, os.WriteFile(path, []byte("a completed fragment"), 0644))
21+
22+
root, err := core.MerkleRoot(path)
23+
require.NoError(t, err)
24+
25+
matching := checkFileExistence(path, root)
26+
require.Error(t, matching)
27+
assert.ErrorIs(t, matching, ErrFileAlreadyExists,
28+
"download_dir matches on this sentinel, so it must stay wrapped rather than replaced")
29+
assert.Contains(t, matching.Error(), path, "the message must say which file")
30+
31+
mismatched := checkFileExistence(path, common.HexToHash("0xdead"))
32+
require.Error(t, mismatched)
33+
assert.NotErrorIs(t, mismatched, ErrFileAlreadyExists, "a different hash is a distinct, fatal error")
34+
assert.Contains(t, mismatched.Error(), "different hash")
35+
assert.Contains(t, mismatched.Error(), path, "the message must say which file")
36+
}
37+
38+
// A path that does not exist is not an error at all.
39+
func TestCheckFileExistence_MissingFileIsNotAnError(t *testing.T) {
40+
assert.NoError(t, checkFileExistence(filepath.Join(t.TempDir(), "absent.dat"), common.HexToHash("0xdead")))
41+
}

0 commit comments

Comments
 (0)