Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

change grpc api errors #53

Merged
merged 1 commit into from
Jul 8, 2020
Merged

change grpc api errors #53

merged 1 commit into from
Jul 8, 2020

Conversation

C0rby
Copy link
Contributor

@C0rby C0rby commented Jul 7, 2020

Add more information to error responses.
I followed the micro example here: https://m3o.com/docs/go-errors.html

Closes #11

@C0rby C0rby requested review from butonic and refs July 7, 2020 14:52
@C0rby C0rby self-assigned this Jul 7, 2020
Copy link
Member

@refs refs left a comment

Choose a reason for hiding this comment

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

Looks good! One thing to consider is if this errors are client facing, since micro client facing errors are not the friendliest, and phoenix won't understand them as far as I understand.

But the diff is great, align errors is a must.

@@ -143,6 +144,7 @@ func New(opts ...Option) (s *Service, err error) {
// TODO don't bother to store fields as we will load the account from disk

s = &Service{
id: cfg.GRPC.Namespace + cfg.Server.Name,
Copy link
Member

Choose a reason for hiding this comment

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

With our default values this will evaluate to com.owncloud.apiaccounts, missing a . between the namespace and the name, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. 👍

Signed-off-by: David Christofas <dchristofas@owncloud.com>
@C0rby C0rby force-pushed the add-error-message-to-responses branch from 6c97de7 to db7afb8 Compare July 8, 2020 09:37
@ownclouders
Copy link

Codacy Here is an overview of what got changed by this pull request:

Clones added
============
- pkg/service/v0/service.go  4
         

See the complete overview on Codacy

@C0rby C0rby merged commit 9d895f9 into master Jul 8, 2020
@delete-merged-branch delete-merged-branch bot deleted the add-error-message-to-responses branch July 8, 2020 12:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add error information to responses
4 participants