Adding error to catch suspended apps #395
Conversation
| if error_details['message'].include? 'suspended application' | ||
| raise Intercom::AppSuspendedError.new(error_details['message'], error_context) | ||
| end | ||
| raise Intercom::AuthenticationError.new(error_details['message'], error_context) |
There was a problem hiding this comment.
Can we move raise AuthenticationError into the else block to make it clearer?
+ if error_details['message'].include? 'suspended application'
+ raise Intercom::AppSuspendedError.new(error_details['message'], error_context)
else
raise Intercom::AuthenticationError.new(error_details['message'], error_context)
+ end
lib/intercom/request.rb
Outdated
| } | ||
| case error_code | ||
| when 'unauthorized', 'forbidden', 'token_not_found' | ||
| if error_details['message'].include? 'suspended application' |
There was a problem hiding this comment.
Do we return suspended application in the response for deleted apps too?
lib/intercom/request.rb
Outdated
| case error_code | ||
| when 'unauthorized', 'forbidden', 'token_not_found' | ||
| raise Intercom::AuthenticationError.new(error_details['message'], error_context) | ||
| if error_details['message'].include? 'suspended application' |
There was a problem hiding this comment.
Do we return suspended application in the response for deleted apps too?
There was a problem hiding this comment.
Yes we do, but I think it might be safer to use the error type instead of the message string!
| case error_code | ||
| when 'unauthorized', 'forbidden', 'token_not_found' | ||
| raise Intercom::AuthenticationError.new(error_details['message'], error_context) | ||
| when 'token_suspended' |
There was a problem hiding this comment.
Do you want add a check for token_unauthorized and token_revoked too?
There was a problem hiding this comment.
Ah good shout, my bad!
lib/intercom/request.rb
Outdated
| case error_code | ||
| when 'unauthorized', 'forbidden', 'token_not_found' | ||
| raise Intercom::AuthenticationError.new(error_details['message'], error_context) | ||
| when 'token_suspended', 'token_revoked', 'token_unauthorized' |
There was a problem hiding this comment.
So my thought here was to introduce two new errors for token_revoked and token_unauthorized. Or we can introduce one new error which is generic that can be TokenError. Your call 👍
jonnyom
force-pushed
the
jonny/suspended_app_error
branch
from
de49836 to
de8bcd8
Compare
No description provided.