search

lineofflight / peddler

Amazon Selling Partner API (SP-API) in Ruby
MIT License
307 stars 130 forks source link

Unknown error due to failure in parsing #136

Closed andrewhavens closed 5 years ago

andrewhavens commented 5 years ago

I noticed an error in our logs:

undefined method `[]' for nil:NilClass

After further investigation, I realized it was happening inside peddler. A call to get_fulfillment_order resulted in an error response from MWS. However, peddler had a problem trying to determine the error code.

/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/errors/parser.rb:18:in `code' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/errors/builder.rb:47:in `error_name' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/errors/builder.rb:39:in `bad_class_name?' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/errors/builder.rb:31:in `build' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/errors/builder.rb:20:in `call' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/client.rb:152:in `handle_http_status_error' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/client.rb:121:in `rescue in run'
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/peddler/client.rb:113:in `run' 
/app/vendor/bundle/ruby/2.5.0/bundler/gems/peddler-73bd11b2d04b/lib/mws/fulfillment_outbound_shipment/client.rb:113:in `get_fulfillment_order'

I looked at the peddler source, and it looks like the problem is that parse returned nil. https://github.com/hakanensari/peddler/blob/master/lib/peddler/errors/parser.rb#L18

Unfortunately, I'm not sure what the response looked like, since the HTTP requests aren't being logged in production.

I'm not sure exactly what the solution should be. Maybe peddler should default to something if it can't determine the error code. If there's some sort of MWS error response logging that I can enable, that would be helpful, but I don't know if such a feature exists.

hakanensari commented 5 years ago

@andrewhavens, thanks for reporting.

This looks like an edge case. I'm adding a guard so we correctly bubble up the original HTTP status error.

About logging, I think it's best if you did this on your end? When hitting MWS, you could simply rescue, log, and re-raise.