search

mapbox / mapbox-sdk-rb

A Ruby interface to Mapbox APIs.
Other
59 stars 32 forks source link

Error Granularity #42

Open LexBartnicki opened 5 years ago

LexBartnicki commented 5 years ago

I am trying to use this gem as a replacement to Geocoder. In my application, if we have trouble communicating with Mapbox when making a request, we rescue that error and perform some fallback behavior.

Unfortunately, any error other than the AuthenticationError that is returned in this gem is a StandardError. So I am left to rescue StandardError which will rescue much more than any error communicating with Mapbox, most of which I would want it to not rescue.

Here you'll see a class called MapboxError, which inherits from StandardError. Can we replace all the instances of StandardError with MapboxError in this file? Then there will at least a catch-all for errors that are related to this SDK.

Eventually I'd hope we could get even more granular and have mapbox-sdk specific errors for every scenario, like they did in the Stripe SDK https://github.com/stripe/stripe-ruby/blob/master/lib/stripe/errors.rb. However, that can come later because for the majority of cases I would assume the difference between rescuing all mapbox-sdk errors and rescuing specific mapbox-sdk errors is a lot smaller than the difference between rescuing all mapbox-sdk errors and rescuing all errors that inherit from StandardError 😅

Please let me know how you want to proceed on this, I would love to hear about Mapbox's process for handling issues in their open-source SDKs.

samfader commented 5 years ago

@LexBartnicki Thanks for your interest in using this SDK! And I love this idea.

To be perfectly honest, there isn't necessarily one maintainer of this repo right now. I work at Mapbox and have an interest in Ruby, so I've been trying to add some smaller features here and there to the SDK, but I'm by no means an expert on it. If you're willing to make some changes and open up a PR, I would be more than happy to help a. review, b. test, and c. deploy a new version of the gem to RubyGems.

rmlrmlrmlrml commented 4 years ago

This is still an issue: I've been dealing with a 422 unprocessable error but the granularity isn't very forgiving: Invalid response object from API: <RestClient::Response 422 "{\"message\":..."> (HTTP response code was 422 Unprocessable Entity). That ... is what I'd really like expanded.

richseviora commented 4 years ago

I had the same issue as @rmlrmlrmlrml where the error was swallowing the details. After debugging I was able to identify that the error message was the coordinate is invalid:

{"message":"Coordinate is invalid: -73.55427,","code":"InvalidInput"}

I'd love to see the source error exposed myself. Since Mapbox provides this SDK I'm kinda expecting that they'll take on correcting this issue.

samfader commented 4 years ago

Thanks for your feedback, @richseviora. Quick point of clarification: Mapbox doesn't provide this SDK - it's open source, and while I work at Mapbox, I do not have the bandwidth to own it. Would you be open to cutting a PR with some added error granularity? Happy to review it or help find someone who can.

waissbluth commented 3 years ago

@rmlrmlrmlrml @richseviora did you find a fix or workaround for this?