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

Report errors to the API #39

Merged
merged 8 commits into from
May 2, 2016
Merged

Report errors to the API #39

merged 8 commits into from
May 2, 2016

Conversation

gabriel
Copy link
Contributor

@gabriel gabriel commented Apr 27, 2016

https://keybase.atlassian.net/browse/DESKTOP-902

I shorted UpdateError to just Error since this is our custom error for package updater.

@gabriel
Copy link
Contributor Author

gabriel commented Apr 27, 2016

@keybase/updater-hackers

https://keybase.atlassian.net/browse/DESKTOP-902

I shorted UpdateError to just Error since this is our custom error for package updater.
@gabriel
Copy link
Contributor Author

gabriel commented Apr 27, 2016

@keybase/updater-hackers bump ready

@@ -57,11 +58,19 @@ func NewUpdater(source UpdateSource, config Config, log logging.Logger) *Updater
}

// Update checks, downloads and performs an update
func (u *Updater) Update(ctx Context) (*Update, error) {
func (u *Updater) Update(ctx Context) (*Update, *Error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why return a *Error rather than an error interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sure, exposing custom error types in a public interface isn't common... and totally unnecessary here

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, this will save you from ever needing an *Error type, which can be buggy because of the crazy nil v nil semantics in Go.

@maxtaco
Copy link
Contributor

maxtaco commented Apr 29, 2016

Big question surrounding the use of an *Error, which I worry could be buggy. Otherwise looks good.

@gabriel
Copy link
Contributor Author

gabriel commented May 2, 2016

I changed around the error helper functions to not return a pointer.. instead using an errPtr function to change Error to pointer for return... basically I'm trying to avoid this:

uerr := cancelErr(err)
return update, &uerr

by doing this:

return update, errPtr(cancelErr(err))

since you can't do return update, &(cancelError(err)) in go

Is this better?

@gabriel
Copy link
Contributor Author

gabriel commented May 2, 2016

Actually, I think I understand... the golang way is the use the error interface throughout and then use a type switch to get a the specific error when needed?

If in reporting, for some reason the error is not out updater.Error type, then the error_type in reporting will be "unknown".

@jzila
Copy link

jzila commented May 2, 2016

@gabriel yeah, error interface + type switch is the way to go.

@gabriel gabriel merged commit 6f650f0 into master May 2, 2016
zanderz pushed a commit that referenced this pull request Jun 20, 2018
* Add git diff

* Add git clean
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.

None yet

3 participants