hudl / fargo

Golang client for Netflix Eureka
MIT License
132 stars 53 forks source link

Ease interpretation of errors that arise during registration operations #45

Closed seh closed 7 years ago

seh commented 8 years ago

At present, the following functions return type error:

They can fail for many reasons, but a few stand out:

To work around these problems, I had to write the following function to filter errors that arise when calling DeregisterInstance:

func deregistrationFailureIsInnocuous(err error) bool {
    msg := err.Error()
    return strings.Contains(msg, "200") || strings.Contains(msg, "404")
}

That is clear evidence of a gap in the interface.

Both ReregisterInstance and RegisterInstance can fail with status code 400 if Eureka deems the instance payload invalid. Note that it's not possible for registration to fail due to a collision with an existing instance with the same ID; Eureka replaces the existing registration.

It would be helpful for callers to be able to discern why these operations failed. Rather than introducing sentinel error values or types, I propose that we introduce a few exported functions:

Do you have counter-proposals for these names? Are there other functions you can think of that would clarify what went wrong?

seh commented 8 years ago

I noticed that fargo already exports type AppNotFoundError. I'd rather not repeat that technique here.

seh commented 7 years ago

See this gist (errors.go) for an adaptation of the interpretive predicate functions I had proposed in #47. You can see that the caller still has consider which operation he had attempted in order to know which function to call, due to Eureka's irregular treatment of metadata updates, but it beats parsing the error messages.

If you change your mind about whether fargo should include these or similar predicates, we can open another issue.