taylorbrooks / closeio

A Ruby wrapper for the Close.io API
http://developer.close.com
MIT License
45 stars 57 forks source link

Error handling v1 #28

Closed rathboma closed 7 years ago

rathboma commented 8 years ago

Hey Taylor,

Just wanted to capture the situation where a bad API key is given. Wanted to run my solution by you. If you think this is good we should add it to the other verbs too.

Let me know if you think this is the right approach.

taylorbrooks commented 8 years ago

I haven't folded this into master yet because it would be a breaking change, but I'd love to get your thoughts on the Error branch.

Check it out here: https://github.com/taylorbrooks/closeio/tree/error

taylorbrooks commented 8 years ago

In particular: https://github.com/taylorbrooks/closeio/blob/error/lib/closeio/error.rb

rathboma commented 8 years ago

I think that's a great way to do it.

I would prepend the error message with something in each of those classes though just so something like this works:

class NotAuthorized < Error
  def initialize(msg)
    this.super("Not Authorized #{msg}")
  end
end

# that way this workflow works:
begin
  model.save
rescue Closeio::Error => e
  model.errors << e.message # this would be something like 'Not Authorized #{env.body}'
end

To make it upgrade friendly just add a initialize variable to let folks turn it off, then the breaking nature of it is a non-issue if you increment the point release:

def initialize(..., throw_errors = true)
   ...
end

...
conn.user(error_handler) if this.throw_errors
rathboma commented 8 years ago

also don't think you need to require faraday in error.rb if you require it in client.rb before requiring error (although not totally sure)

rathboma commented 8 years ago

Hey @taylorbrooks, is there an update on this branch? You planning to merge it? I'd love to use these features officially!

taylorbrooks commented 8 years ago

@rathboma I just cut a new version 2.5.0 that uses the error handler middleware.

It is disabled by default, but can be enabled by passing true to the fourth argument when instantiating the client.

def initialize(api_key, logger = true, ca_file = nil, errors = false)
  # stuff
end

I'm thinking on cutting a 3.0.0 series that uses keyword args for the client and some of the underlying endpoints. Thoughts?

taylorbrooks commented 8 years ago

Ok to close?

rathboma commented 7 years ago

Hey, thought I'd check in to see if error handling made it into any releases? I've been getting 400 errors and I'm not picking them up really

taylorbrooks commented 7 years ago

@rathboma https://github.com/taylorbrooks/closeio/blob/master/lib/closeio/client.rb#L27-L32