From c7f847c96eecf68f7e27fbdf93a7fb59f770289a Mon Sep 17 00:00:00 2001 From: Darien Raymond Date: Wed, 24 Oct 2018 12:02:02 +0200 Subject: [PATCH] categorize read and write error --- common/buf/copy.go | 56 ++++++++++++++++---------- common/buf/copy_test.go | 54 ++++++++++++++++++++++++++ common/buf/writer_test.go | 4 +- mocks.go | 2 + testing/mocks/io.go | 82 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 177 insertions(+), 21 deletions(-) create mode 100644 common/buf/copy_test.go create mode 100644 testing/mocks/io.go diff --git a/common/buf/copy.go b/common/buf/copy.go index fb19e2e24..1d301b168 100644 --- a/common/buf/copy.go +++ b/common/buf/copy.go @@ -45,24 +45,6 @@ type SizeCounter struct { // CopyOption is an option for copying data. type CopyOption func(*copyHandler) -// IgnoreReaderError is a CopyOption that ignores errors from reader. Copy will continue in such case. -func IgnoreReaderError() CopyOption { - return func(handler *copyHandler) { - handler.onReadError = append(handler.onReadError, func(err error) error { - return nil - }) - } -} - -// IgnoreWriterError is a CopyOption that ignores errors from writer. Copy will continue in such case. -func IgnoreWriterError() CopyOption { - return func(handler *copyHandler) { - handler.onWriteError = append(handler.onWriteError, func(err error) error { - return nil - }) - } -} - // UpdateActivity is a CopyOption to update activity on each data copy operation. func UpdateActivity(timer signal.ActivityUpdater) CopyOption { return func(handler *copyHandler) { @@ -81,6 +63,40 @@ func CountSize(sc *SizeCounter) CopyOption { } } +type readError struct { + error +} + +func (e readError) Error() string { + return e.error.Error() +} + +func (e readError) Inner() error { + return e.error +} + +func IsReadError(err error) bool { + _, ok := err.(readError) + return ok +} + +type writeError struct { + error +} + +func (e writeError) Error() string { + return e.error.Error() +} + +func (e writeError) Inner() error { + return e.error +} + +func IsWriteError(err error) bool { + _, ok := err.(writeError) + return ok +} + func copyInternal(reader Reader, writer Writer, handler *copyHandler) error { for { buffer, err := handler.readFrom(reader) @@ -90,12 +106,12 @@ func copyInternal(reader Reader, writer Writer, handler *copyHandler) error { } if werr := handler.writeTo(writer, buffer); werr != nil { - return werr + return writeError{werr} } } if err != nil { - return err + return readError{err} } } } diff --git a/common/buf/copy_test.go b/common/buf/copy_test.go new file mode 100644 index 000000000..0467b231b --- /dev/null +++ b/common/buf/copy_test.go @@ -0,0 +1,54 @@ +package buf_test + +import ( + "crypto/rand" + "testing" + + "github.com/golang/mock/gomock" + + "v2ray.com/core/common/buf" + "v2ray.com/core/common/errors" + "v2ray.com/core/testing/mocks" +) + +func TestReadError(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockReader := mocks.NewReader(mockCtl) + mockReader.EXPECT().Read(gomock.Any()).Return(0, errors.New("error")) + + err := buf.Copy(buf.NewReader(mockReader), buf.Discard) + if err == nil { + t.Fatal("expected error, but nil") + } + + if !buf.IsReadError(err) { + t.Error("expected to be ReadError, but not") + } + + if err.Error() != "error" { + t.Fatal("unexpected error message: ", err.Error()) + } +} + +func TestWriteError(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockWriter := mocks.NewWriter(mockCtl) + mockWriter.EXPECT().Write(gomock.Any()).Return(0, errors.New("error")) + + err := buf.Copy(buf.NewReader(rand.Reader), buf.NewWriter(mockWriter)) + if err == nil { + t.Fatal("expected error, but nil") + } + + if !buf.IsWriteError(err) { + t.Error("expected to be WriteError, but not") + } + + if err.Error() != "error" { + t.Fatal("unexpected error message: ", err.Error()) + } +} diff --git a/common/buf/writer_test.go b/common/buf/writer_test.go index 270aabfb8..7adcf610a 100644 --- a/common/buf/writer_test.go +++ b/common/buf/writer_test.go @@ -41,7 +41,9 @@ func TestBytesWriterReadFrom(t *testing.T) { writer.SetBuffered(false) nBytes, err := reader.WriteTo(writer) assert(nBytes, Equals, int64(size)) - assert(err, IsNil) + if err != nil { + t.Fatal("expect success, but actually error: ", err.Error()) + } mb, err := pReader.ReadMultiBuffer() assert(err, IsNil) diff --git a/mocks.go b/mocks.go index 743ef8bdd..6c975e279 100644 --- a/mocks.go +++ b/mocks.go @@ -2,5 +2,7 @@ package core //go:generate go get -u github.com/golang/mock/gomock //go:generate go install github.com/golang/mock/mockgen + +//go:generate mockgen -package mocks -destination testing/mocks/io.go -mock_names Reader=Reader,Writer=Writer io Reader,Writer //go:generate mockgen -package mocks -destination testing/mocks/dns.go -mock_names Client=DNSClient v2ray.com/core/features/dns Client //go:generate mockgen -package mocks -destination testing/mocks/proxy.go -mock_names Inbound=ProxyInbound,Outbound=ProxyOutbound v2ray.com/core/proxy Inbound,Outbound diff --git a/testing/mocks/io.go b/testing/mocks/io.go new file mode 100644 index 000000000..8346301cf --- /dev/null +++ b/testing/mocks/io.go @@ -0,0 +1,82 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: io (interfaces: Reader,Writer) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// Reader is a mock of Reader interface +type Reader struct { + ctrl *gomock.Controller + recorder *ReaderMockRecorder +} + +// ReaderMockRecorder is the mock recorder for Reader +type ReaderMockRecorder struct { + mock *Reader +} + +// NewReader creates a new mock instance +func NewReader(ctrl *gomock.Controller) *Reader { + mock := &Reader{ctrl: ctrl} + mock.recorder = &ReaderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *Reader) EXPECT() *ReaderMockRecorder { + return m.recorder +} + +// Read mocks base method +func (m *Reader) Read(arg0 []byte) (int, error) { + ret := m.ctrl.Call(m, "Read", arg0) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Read indicates an expected call of Read +func (mr *ReaderMockRecorder) Read(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Read", reflect.TypeOf((*Reader)(nil).Read), arg0) +} + +// Writer is a mock of Writer interface +type Writer struct { + ctrl *gomock.Controller + recorder *WriterMockRecorder +} + +// WriterMockRecorder is the mock recorder for Writer +type WriterMockRecorder struct { + mock *Writer +} + +// NewWriter creates a new mock instance +func NewWriter(ctrl *gomock.Controller) *Writer { + mock := &Writer{ctrl: ctrl} + mock.recorder = &WriterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *Writer) EXPECT() *WriterMockRecorder { + return m.recorder +} + +// Write mocks base method +func (m *Writer) Write(arg0 []byte) (int, error) { + ret := m.ctrl.Call(m, "Write", arg0) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Write indicates an expected call of Write +func (mr *WriterMockRecorder) Write(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Write", reflect.TypeOf((*Writer)(nil).Write), arg0) +}