GODRIVER-2704 Replace input validation error constants with "InvalidArgumentError". #1989
Conversation
API Change Report./v2/mongocompatible changesInvalidArgumentError: added |
mongo/errors.go
Outdated
| var ErrMultipleIndexDrop = InvalidArgumentError{errors.New("multiple indexes would be dropped")} | ||
|
|
||
| // ErrNilDocument is returned when a nil document is passed to a CRUD method. | ||
| var ErrNilDocument = errors.New("document is nil") | ||
| var ErrNilDocument = InvalidArgumentError{errors.New("document is nil")} | ||
|
|
||
| // ErrNilValue is returned when a nil value is passed to a CRUD method. | ||
| var ErrNilValue = errors.New("value is nil") | ||
| var ErrNilValue = InvalidArgumentError{errors.New("value is nil")} | ||
|
|
||
| // ErrEmptySlice is returned when an empty slice is passed to a CRUD method that requires a non-empty slice. | ||
| var ErrEmptySlice = errors.New("must provide at least one element in input slice") | ||
| var ErrEmptySlice = InvalidArgumentError{errors.New("must provide at least one element in input slice")} | ||
|
|
||
| // ErrNotSlice is returned when a type other than slice is passed to InsertMany. | ||
| var ErrNotSlice = errors.New("must provide a non-empty slice") | ||
| var ErrNotSlice = InvalidArgumentError{errors.New("must provide a non-empty slice")} |
There was a problem hiding this comment.
We should keep the type of these error values error to preserve backward compatibility.
E.g.
var ErrNotSlice error = InvalidArgumentError{errors.New("must provide a non-empty slice")}| @@ -31,9 +31,6 @@ var ErrInvalidIndexValue = errors.New("invalid index value") | |||
| // ErrNonStringIndexName is returned if an index is created with a name that is not a string. | |||
| var ErrNonStringIndexName = errors.New("index name must be a string") | |||
There was a problem hiding this comment.
This error value appears to be unused. Can we mark it as deprecated, or should it be returned in some case?
There was a problem hiding this comment.
It seems not being used in the current v1 code base either. Is removing it considered a breaking change?
There was a problem hiding this comment.
Yeah, removing it is technically breaking because someone's code might reference the symbol, which would cause a compilation error if we remove it. The best we can do is mark it deprecated and note that it's no longer used.
mongo/client_bulk_write.go
Outdated
| @@ -50,11 +50,11 @@ type clientBulkWrite struct { | |||
|
|
|||
| func (bw *clientBulkWrite) execute(ctx context.Context) error { | |||
| if len(bw.writePairs) == 0 { | |||
| return ErrEmptySlice | |||
| return fmt.Errorf("error from writes: %w", ErrEmptySlice) | |||
There was a problem hiding this comment.
Optional: Consider a more descriptive error message.
E.g.
fmt.Errorf("empty writes slice: %w", ErrEmptySlice)There was a problem hiding this comment.
I think "empty writes slice" is redundant because the message in ErrEmptySlice has stated the slice should be non-empty.
There was a problem hiding this comment.
My goal is to clarify which BulkWrite parameter caused the validation error. "error from writes" is somewhat ambiguous because it could be interpreted as meaning there was an error performing the writes on the server. Would a less redundant error like "invalid writes" be better?
GODRIVER-2704
Summary
Replace input validation error constants with "InvalidArgumentError".
Background & Motivation