zmb3 / spotify

A Go wrapper for the Spotify Web API
Apache License 2.0
1.4k stars 295 forks source link

Expose RetryAfter value in error #274

Open flimzy opened 1 week ago

flimzy commented 1 week ago

Related to, and useful along with #273

Pineapple217 commented 1 week ago

IMO the Error type should contain time.Duration instead of time.Time. If the user wants time.Time they can calculate it them self (or add a method to Error to calculate it on the fly if you want it in the lib). Because if you want time.Duration you are just calculating back the value (doing double work). I know its nitpicky but something to think about. Either way I would be happy with this feature.

flimzy commented 1 week ago

IMO the Error type should contain time.Duration instead of time.Time.

That was my initial thought as well. The problem with a time.Duration is that it looses its meaning virtually immediately. If you're doing any sort of batch processing, or anything else that might delay the handling of an error, then that value cannot be relied on any more. By returning a time.Time, we aren't forcing the consumer of this library into any specific error handling behavior or pattern.

Pineapple217 commented 1 week ago

Fair enough, both have their pros and cons but time.Time is probably better fitting for this lib when you put it like that. More intuitive and flexible.