Shippo::Manifest.create obscures invalid transaction error with Shippo::Exceptions::ConnectionError

[oehlschl]

We just encountered this regression after upgrading from version 1.0 to version 3.0 of the Shippo gem/API. It looks like this was really introduced in version 2.0* and we can work around it, but it's still worth noting that the behavior is unexpected and different than Shippo clients in other languages.

Previously, invalid transactions sent to Shippo::Manifest.create were returned in the message body of a Shippo::APIError object. Now, however, a manifest request with invalid transactions results yields an underlying RestClient::NotFound exception, which is wrapped by a Shippo::Exceptions::ConnectionError here:

shippo-ruby-client/lib/shippo/api/request.rb

Line 75 in a36a024

rescue ::RestClient::Exception => e

. The original exception message containing the invalid transactions is obscured, and, in particular, the connection error is misleading. Ideally, the actual error message would still appear accessible to the end user.

It looks like at least the PHP and Java clients treat 404s as invalid requests; see
https://github.com/goshippo/shippo-php-client/blob/master/lib/Shippo/ApiRequestor.php#L102 and https://github.com/goshippo/shippo-java-client/blob/5ee5e3068742640cc827d23ab709a5b364c8b613/src/main/java/com/shippo/net/APIResource.java#L522.

Can we we also include NotFound with the BadRequest logic here? https://github.com/goshippo/shippo-ruby-client/blob/master/lib/shippo/api/request.rb#L72

*The fallback exception class changed to ConnectionError in 2.0 (https://github.com/goshippo/shippo-ruby-client/blob/v2.0/lib/shippo/api/request.rb#L64), and the error message is no longer passed through as it was in 1.0. (https://github.com/goshippo/shippo-ruby-client/blob/v1.0.4/lib/shippo.rb#L88)

[oehlschl]

It looks like ::RestClient::BadRequest is also rescued twice (without being reraised), on both

shippo-ruby-client/lib/shippo/api/request.rb

Line 62 in a36a024

rescue ::RestClient::BadRequest => e

and

shippo-ruby-client/lib/shippo/api/request.rb

Line 72 in a36a024

rescue ::RestClient::BadRequest => e

[oehlschl]

It actually looks like InvalidInputError doesn't exist, possibly missed because the rescue code path is never hit. https://github.com/goshippo/shippo-ruby-client/search?utf8=%E2%9C%93&q=InvalidInputError&type=

[jfriedr]

@oehlschl I'm going to attempt to sift through all our outstanding issues and pull requests.

Are you still seeing this behavior or can this issue be closed out?