go-sdk
go-sdk copied to clipboard
Error Handling Bugs
Hi, while trying out the SDK I noticed some instances of incomplete error handling. This isn't a very comprehensive list; it's just what I saw while looking at the actor code primarily.
Bugs
- [ ]
NewClient()can return anilerror on a repeat invocation, because theonceErris in the function scope. (#280) - [ ]
ImplActorClientStub()and its childimplActor()do not notify the caller if an error was encountered. Maybe add a new method with anerrorreturn type, or change it topanic()on errors for now. - [ ]
RegisterActorFactory()ignores theNewDefaultActorManager()error and returnsnilto the caller without even logging. - [ ] Invoking an actor method on a client stub can hit an error condition and not notify the caller.
- Example of an error that is logged without being returned to the caller: https://github.com/dapr/go-sdk/blob/7c38f5a607d58ee0af93a2f26a061d5c1834203a/client/actor.go#L392-L393
- Example of an error which is an ineffectual assignment. The
errvariable is unused after this point: https://github.com/dapr/go-sdk/blob/7c38f5a607d58ee0af93a2f26a061d5c1834203a/client/actor.go#L404
- [ ] The http service implementation does not log the error if one was encountered (for example, an issue with a custom serializer), making debugging difficult.
Other areas for improvement
- I noticed you are using the github.com/pkg/errors package for creating and wrapping error types. Since Go 1.13, error wrapping is built into the standard
errorsandfmtlibraries, so you could remove this dependency. - Some of these bugs can be caught and enforced with a linter. I recommend golangci-lint, which has
errcheckandineffassignenabled by default.
I think it could be worth segregating them as separate issues so that they can be PR´d individually.