mrkt icon indicating copy to clipboard operation
mrkt copied to clipboard

Add middleware to handle unexpected non-json response for edge case

Open nikushi opened this issue 6 years ago • 1 comments

Hi, recently I got NoMethodError exception during Marketo side system trouble. This exception happened on GET /identity/oauth/token due to following backtrace.

got #<NoMethodError: undefined method `fetch' for String> with backtrace:
             # ./lib/mrkt/concerns/authentication.rb:34:in `block in authenticate'
             # ./lib/mrkt/concerns/authentication.rb:31:in `tap'
             # ./lib/mrkt/concerns/authentication.rb:31:in `authenticate'

This is a bug of mrkt gem of edge case handling. There is a case that Marketo's server could respond with non json content-type e.g. text/plain. Although I could not confirm what it's content type was accurately, I believe that the exception was caused by the content type, from the stracktrace we got. You can see the detail in the first commit to reproduce the bug.

nikushi avatar Nov 30 '18 03:11 nikushi

Thanks for reviewing. Do you mean that the middleware I added should not pass 2XX response? Instead it should raise the same error too? I was worried about that point actually. If we choose more strict approach, it should be raised if the middleware see a response type other than json, even if it's status is 2XX or something successful one.

nikushi avatar Dec 03 '18 03:12 nikushi