algorithmia-ruby icon indicating copy to clipboard operation
algorithmia-ruby copied to clipboard

Error messages in API response are not surfaced by Requester

Open denzel-morris opened this issue 6 years ago • 0 comments

Problem Description

When an algorithm developer attempts to invoke another algorithm with an improperly configured client:

client = Algorithmia.client('<api key>')
client.algo('example/algo').pipe(input)

The API returns an appropriately descriptive error message in its response:

{
    "error": {
        "message": "invalid authorization: api keys are not allowed within algorithms"
    }
}

That tells the algorithm developer that API keys are not to be used within algorithms.

However, the algorithmia-ruby library does not surface this message. Instead, it raises an exception with a rather misleading message:

Error: The request you are making requires authorization. Please check that you have permissions & that you've set your API key.
/opt/algorithm/vendor/bundle/ruby/2.2.0/gems/algorithmia-1.0.1/lib/algorithmia/requester.rb:92:in `check_for_errors'
# ...

Cause

Requester#check_for_errors intends to surface the API response error message, as can be seen on line 87:

message = response.dig("error", "message") if response.is_a?(Hash)

However, the response object returned is a HTTParty::Response object. This HTTParty::Response object is returned by the delegated get, post, put, head, and delete within Requester

Note that while the HTTParty::Response responds to Hash-like methods due to internal method_missing delegation, is_a? Hash will evaluate to false.

Additional Concern

Even if we were to remove a Hash type check, we must be careful. Hash#dig was added in Ruby 2.3; and the Algorithmia developer platform runs on Ruby 2.2. Therefore Hash#dig is not available to algorithm developers and will raise an error if invoked within an algorithm.

Solution

There are a few possible solutions. First, we should remove the Hash type check. Additionally, we should either find a way other than Hash#dig to retrieve the error message, or we should specify a required ruby version in the gem specification for this library.

Also, it may make sense to add a regression test to exercise this code path such that it doesn't resurface in the future.

denzel-morris avatar Aug 20 '18 06:08 denzel-morris