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

Model good error handling practices in examples #70

Open
drstrangelooker opened this issue Jun 12, 2018 · 11 comments
Open

Model good error handling practices in examples #70

drstrangelooker opened this issue Jun 12, 2018 · 11 comments

Comments

@drstrangelooker
Copy link
Contributor

So I was working with a customer using the API who was struggling because the errors were not explanatory. I tried their call in the API web page and got back a detailed useful error message. I recommended that the user change their code to catch the error and display a useful error message.

This kind of pattern should be modeled in the examples.

begin
  req = {
    :name => group_name,
    :label => group_name,
    :type => 'string',
    :default_value => group_name,
    :user_can_view => false,
    :user_can_edit => false
  }
  attribute = sdk.create_user_attribute(req)
rescue LookerSDK::Error => e
  puts e.message
  puts e.errors if e.errors
  raise
end
@jbandhauer
Copy link
Contributor

I'd also recommend adding puts e.class. Often without that context the message can be confusing.

@jbandhauer
Copy link
Contributor

... and some generic info that an exception was raised calling whatever can be helpful if you use something like this as boilerplate around other calls. Otherwise it is just printing some stuff having to do with who-knows-what.

@drstrangelooker
Copy link
Contributor Author

For the short examples like this, the raise makes sure that the call stack is printed so the developer can have a clue of what is happening where. For a more fully fledged application, absolutely.

@jbandhauer
Copy link
Contributor

jbandhauer commented Jun 12, 2018

OK, sure. But, you didn't show something that is necessarily a full program; you showed a snippet, so no telling what code is gonna eat the exception if someone integrates that into their larger program. Just say'n ;)

@jakemitchellxyz
Copy link

+1 - I still have no clue how to catch errors with the SDK (aside from the generic catch above). It should be very clear in the readme or in the Docs or API reference (or all 3) how to check the status of an API call from the SDK. For my use case, I just need to know whether the call returned 200 or something else, but I don't know how to do that without looking at the SDK code. If I wanted to handle 409 in a special way, I (as a user) don't know how to check that.

@jbandhauer
Copy link
Contributor

I fully agree that better docs would be great. Lacking that... 2xx responses simply return results, Other stuff throws exceptions as per the mapping in https://github.com/looker/looker-sdk-ruby/blob/master/lib/looker-sdk/error.rb#L14-L29

@drstrangelooker
Copy link
Contributor Author

If you look at https://github.com/looker/looker-sdk-ruby/blob/master/lib/looker-sdk/error.rb you’ll see the various errors you can catch.

LookerSDK::Error catches any 4xx or 5xx return.
LookerSDK:ClientError catches any 4xx return.
LookerSDK:ServerError catches any 5xx return.
LookerSDK::NotFound for example, catches a 404. You’ll need to look at that file to see all the other specific error classes.

So typically you might want to do something like...

begin
  # API code here
rescue LookerSDK::NotFound => e
  # handling for not found
  # This may not even be a real error case
rescue LookerSDK::ClientError => e
  # handling for any other client error
  # you can probably log a useful message
rescue LookerSDK::Error => e
  # hopefully should never get here, probably can’t recover
  # provide lots of logging and a stack trace
end

@jbandhauer
Copy link
Contributor

I suspect that for most SDK use cases one wants to assume success and just report the errors (and fail) and not be highly concerned about specially processing specific error types - unless you are really writing something that knows exactly how to logically handle them and can work around any failures. That is why they are modeled as exceptions and not as a pattern that requires explicit checking of result code after each API call. Pardon the pun, but I think scripts that really know how to handle such errors without simply failing are the exceptional case.

@jakemitchellxyz
Copy link

That's great! Yeah, I saw that file, but people using the SDK shouldn't have to read the code to see how to handle errors. The use case @deangelo-llooker just showed is fantastic, I think that should go in the docs or somewhere more visible. I agree the exhaustive list of classes can stay hiding in the code though, like you said, I don't think many people will need to use that very often

@drstrangelooker
Copy link
Contributor Author

drstrangelooker commented Jun 26, 2018 via email

@jakemitchellxyz
Copy link

Also for context, in my use case, if I get a LookerSDK::Conflict, I can just pretend nothing happened, but any other error I need to log and fail

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants