Fix exception: NoMethodError: "undefined method `status' for #<Faraday::TimeoutError:...>"#81
Fix exception: NoMethodError: "undefined method `status' for #<Faraday::TimeoutError:...>"#81kapcod wants to merge 1 commit intomhenrixon:masterfrom
Conversation
…y::TimeoutError:...>" Somehow Faraday::TimeoutError object is passed as `response` parameter. In this case it doesn't have status or body, but the request url/headers still matter. Probably the bug is in a different place that passes the invalid `response` parameter, but even if that bug will be found and fixed, this change can stay as a failsafe.
| URL: #{env.url} | ||
| REQUEST HEADERS: #{env.request_headers} | ||
| RESPONSE_HEADERS: #{env.response_headers} | ||
| REQUEST_BODY: #{env.request_body}\n\n" |
There was a problem hiding this comment.
ActiveCampaign::Error#message calls 'env.request_body' 2 times
| <<~MESSAGE | ||
| ERROR: #{response.class.name}: #{response} | ||
| URL: #{env.url} | ||
| REQUEST HEADERS: #{env.request_headers} |
There was a problem hiding this comment.
ActiveCampaign::Error#message calls 'env.request_headers' 2 times
| ERROR: #{response.class.name}: #{response} | ||
| URL: #{env.url} | ||
| REQUEST HEADERS: #{env.request_headers} | ||
| RESPONSE_HEADERS: #{env.response_headers} |
There was a problem hiding this comment.
ActiveCampaign::Error#message calls 'env.response_headers' 2 times
| elsif response.is_a?(Exception) | ||
| <<~MESSAGE | ||
| ERROR: #{response.class.name}: #{response} | ||
| URL: #{env.url} |
There was a problem hiding this comment.
ActiveCampaign::Error#message calls 'env.url' 2 times
| URL: #{env.url} | ||
| REQUEST HEADERS: #{env.request_headers} | ||
| RESPONSE_HEADERS: #{env.response_headers} | ||
| REQUEST_BODY: #{env.request_body}\n\n" |
There was a problem hiding this comment.
ActiveCampaign::Error#message calls 'env.request_body' 2 times
|
Code Climate has analyzed commit 4950836 and detected 9 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
+1 any possibility to merge this @mhenrixon? |
| def message | ||
| if response.nil? | ||
| super | ||
| elsif response.is_a?(Exception) |
There was a problem hiding this comment.
Would it make sense to be a little more specific?
Also, there are a bunch of duplication now that code climate warns about. Would it perhaps make sense to address that? I am hesitant to merge something that doesn't pass the rubocop style guide.
Somehow Faraday::TimeoutError object is passed as
responseparameter. In this case it doesn't have status or body, but the request url/headers still matter.Probably the bug is in a different place that passes the invalid
responseparameter, but even if that bug will be found and fixed, this change can stay as a failsafe.