Skip to content
This repository has been archived by the owner on Jun 19, 2023. It is now read-only.

feat: use error channels #62

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion block.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package iface

import (
"context"
path "github.com/ipfs/interface-go-ipfs-core/path"
"io"

"github.com/ipfs/interface-go-ipfs-core/options"
path "github.com/ipfs/interface-go-ipfs-core/path"
)

// BlockStat contains information about a block
Expand Down
7 changes: 1 addition & 6 deletions name.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ type IpnsEntry interface {
Value() path.Path
}

type IpnsResult struct {
path.Path
Err error
}

// NameAPI specifies the interface to IPNS.
//
// IPNS is a PKI namespace, where names are the hashes of public keys, and the
Expand All @@ -43,5 +38,5 @@ type NameAPI interface {
//
// Note: by default, all paths read from the channel are considered unsafe,
// except the latest (last path in channel read buffer).
Search(ctx context.Context, name string, opts ...options.NameResolveOption) (<-chan IpnsResult, error)
Search(ctx context.Context, name string, opts ...options.NameResolveOption) (<-chan path.Path, <-chan error)
}
7 changes: 2 additions & 5 deletions pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ type Pin interface {

// Type of the pin
Type() string

// if not nil, an error happened. Everything else should be ignored.
Err() error
}

// PinStatus holds information about pin health
Expand Down Expand Up @@ -44,7 +41,7 @@ type PinAPI interface {
Add(context.Context, path.Path, ...options.PinAddOption) error

// Ls returns list of pinned objects on this node
Ls(context.Context, ...options.PinLsOption) (<-chan Pin, error)
Ls(context.Context, ...options.PinLsOption) (<-chan Pin, <-chan error)

// IsPinned returns whether or not the given cid is pinned
// and an explanation of why its pinned
Expand All @@ -58,5 +55,5 @@ type PinAPI interface {
Update(ctx context.Context, from path.Path, to path.Path, opts ...options.PinUpdateOption) error

// Verify verifies the integrity of pinned objects
Verify(context.Context) (<-chan PinStatus, error)
Verify(context.Context) (<-chan PinStatus, <-chan error)
}
21 changes: 6 additions & 15 deletions tests/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,17 @@ func (tp *TestSuite) TestPinRecursive(t *testing.T) {
t.Errorf("unexpected path, %s != %s", list[0].Path().Cid().String(), p0.Cid().String())
}

res, err := api.Pin().Verify(ctx)
if err != nil {
t.Fatal(err)
}
res, errCh := api.Pin().Verify(ctx)
n := 0
for r := range res {
if !r.Ok() {
t.Error("expected pin to be ok")
}
n++
}
if err := <-errCh; err != nil {
t.Fatal(err)
}

if n != 1 {
t.Errorf("unexpected verify result count: %d", n)
Expand Down Expand Up @@ -583,19 +583,10 @@ func assertNotPinned(t *testing.T, ctx context.Context, api iface.CoreAPI, p pat
}
}

func accPins(pins <-chan iface.Pin, err error) ([]iface.Pin, error) {
if err != nil {
return nil, err
}

func accPins(pins <-chan iface.Pin, err <-chan error) ([]iface.Pin, error) {
var result []iface.Pin

for pin := range pins {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, when an error happen, the result channel is not closed right ? Doesn't that means the caller would get stuck in this reading loop ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new design, both the result and the error channels are always closed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I need to document that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the issue description and the interface documentation. It looks like docker isn't actually all that consistent.

if pin.Err() != nil {
return nil, pin.Err()
}
result = append(result, pin)
}

return result, nil
return result, <-err
}
48 changes: 26 additions & 22 deletions tests/unixfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,15 +681,9 @@ func (tp *TestSuite) TestLs(t *testing.T) {
t.Fatal(err)
}

entries, err := api.Unixfs().Ls(ctx, p)
if err != nil {
t.Fatal(err)
}
entries, errCh := api.Unixfs().Ls(ctx, p)

entry := <-entries
if entry.Err != nil {
t.Fatal(entry.Err)
}
if entry.Size != 15 {
t.Errorf("expected size = 15, got %d", entry.Size)
}
Expand All @@ -703,9 +697,6 @@ func (tp *TestSuite) TestLs(t *testing.T) {
t.Errorf("expected cid = QmX3qQVKxDGz3URVC3861Z3CKtQKGBn6ffXRBBWGMFz9Lr, got %s", entry.Cid)
}
entry = <-entries
if entry.Err != nil {
t.Fatal(entry.Err)
}
if entry.Type != coreiface.TSymlink {
t.Errorf("wrong type %s", entry.Type)
}
Expand All @@ -716,11 +707,11 @@ func (tp *TestSuite) TestLs(t *testing.T) {
t.Errorf("expected symlink target to be /foo/bar, got %s", entry.Target)
}

if l, ok := <-entries; ok {
if _, ok := <-entries; ok {
t.Errorf("didn't expect a second link")
if l.Err != nil {
t.Error(l.Err)
}
}
if err := <-errCh; err != nil {
t.Error(err)
}
}

Expand Down Expand Up @@ -784,14 +775,20 @@ func (tp *TestSuite) TestLsEmptyDir(t *testing.T) {
t.Fatal(err)
}

links, err := api.Unixfs().Ls(ctx, path.IpfsPath(emptyDir.Cid()))
if err != nil {
t.Fatal(err)
links, errCh := api.Unixfs().Ls(ctx, path.IpfsPath(emptyDir.Cid()))
count := 0
for range links {
count++
}

if len(links) != 0 {
if count != 0 {
t.Fatalf("expected 0 links, got %d", len(links))
}

err = <-errCh
if err != nil {
t.Fatal(err)
}
}

// TODO(lgierth) this should test properly, with len(links) > 0
Expand All @@ -813,14 +810,21 @@ func (tp *TestSuite) TestLsNonUnixfs(t *testing.T) {
t.Fatal(err)
}

links, err := api.Unixfs().Ls(ctx, path.IpfsPath(nd.Cid()))
if err != nil {
t.Fatal(err)
links, errCh := api.Unixfs().Ls(ctx, path.IpfsPath(nd.Cid()))

count := 0
for range links {
count++
}

if len(links) != 0 {
if count != 0 {
t.Fatalf("expected 0 links, got %d", len(links))
}

err = <-errCh
if err != nil {
t.Fatal(err)
}
}

type closeTestF struct {
Expand Down
4 changes: 1 addition & 3 deletions unixfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ type DirEntry struct {
Size uint64 // The size of the file in bytes (or the size of the symlink).
Type FileType // The type of the file.
Target string // The symlink target (if a symlink).

Err error
}

// UnixfsAPI is the basic interface to immutable files in IPFS
Expand All @@ -75,5 +73,5 @@ type UnixfsAPI interface {

// Ls returns the list of links in a directory. Links aren't guaranteed to be
// returned in order
Ls(context.Context, path.Path, ...options.UnixfsLsOption) (<-chan DirEntry, error)
Ls(context.Context, path.Path, ...options.UnixfsLsOption) (<-chan DirEntry, <-chan error)
}
20 changes: 0 additions & 20 deletions util.go

This file was deleted.