From c8b81b066b64a3436b55f1ebf570e6c393cd4380 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 31 Mar 2021 16:39:57 +0530 Subject: [PATCH] chore: Add timeouts to requests (#2024) --- .../conversation_notifications_mailer.rb | 6 +++--- lib/local_resource.rb | 3 ++- lib/webhooks/trigger.rb | 7 ++++++- spec/lib/webhooks/trigger_spec.rb | 13 ++++++++++--- .../conversation_notifications_mailer_spec.rb | 5 +++-- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/app/mailers/agent_notifications/conversation_notifications_mailer.rb b/app/mailers/agent_notifications/conversation_notifications_mailer.rb index 60e5401da..877c68ab5 100644 --- a/app/mailers/agent_notifications/conversation_notifications_mailer.rb +++ b/app/mailers/agent_notifications/conversation_notifications_mailer.rb @@ -29,13 +29,13 @@ class AgentNotifications::ConversationNotificationsMailer < ApplicationMailer send_mail_with_liquid(to: @agent.email, subject: subject) and return end - def assigned_conversation_new_message(conversation, agent) + def assigned_conversation_new_message(message, agent) return unless smtp_config_set_or_development? # Don't spam with email notifications if agent is online - return if ::OnlineStatusTracker.get_presence(conversation.account.id, 'User', agent.id) + return if ::OnlineStatusTracker.get_presence(message.account_id, 'User', agent.id) @agent = agent - @conversation = conversation + @conversation = message.conversation subject = "#{@agent.available_name}, New message in your assigned conversation [ID - #{@conversation.display_id}]." @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) send_mail_with_liquid(to: @agent.email, subject: subject) and return diff --git a/lib/local_resource.rb b/lib/local_resource.rb index 530861ca8..e7fa28106 100644 --- a/lib/local_resource.rb +++ b/lib/local_resource.rb @@ -16,7 +16,8 @@ class LocalResource end def io - @io ||= uri.open + # TODO: should we use RestClient here too ? + @io ||= uri.open(read_timeout: 5) end def encoding diff --git a/lib/webhooks/trigger.rb b/lib/webhooks/trigger.rb index c3f0f95f2..8bbaa05a4 100644 --- a/lib/webhooks/trigger.rb +++ b/lib/webhooks/trigger.rb @@ -1,6 +1,11 @@ class Webhooks::Trigger def self.execute(url, payload) - RestClient.post(url, payload.to_json, { content_type: :json, accept: :json }) + RestClient::Request.execute( + method: :post, + url: url, payload: payload.to_json, + headers: { content_type: :json, accept: :json }, + timeout: 5 + ) rescue *ExceptionList::REST_CLIENT_EXCEPTIONS, *ExceptionList::URI_EXCEPTIONS => e Rails.logger.info "Exception: invalid webhook url #{url} : #{e.message}" rescue StandardError => e diff --git a/spec/lib/webhooks/trigger_spec.rb b/spec/lib/webhooks/trigger_spec.rb index 7608634ac..b5a2dcbd2 100644 --- a/spec/lib/webhooks/trigger_spec.rb +++ b/spec/lib/webhooks/trigger_spec.rb @@ -5,11 +5,18 @@ describe Webhooks::Trigger do describe '#execute' do it 'triggers webhook' do - params = { hello: :hello } + payload = { hello: :hello } url = 'https://test.com' - expect(RestClient).to receive(:post).with(url, params.to_json, { accept: :json, content_type: :json }).once - trigger.execute(url, params) + expect(RestClient::Request).to receive(:execute) + .with( + method: :post, + url: url, + payload: payload.to_json, + headers: { content_type: :json, accept: :json }, + timeout: 5 + ).once + trigger.execute(url, payload) end end end diff --git a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb index d44824892..3f909fb97 100644 --- a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb +++ b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb @@ -52,10 +52,11 @@ RSpec.describe AgentNotifications::ConversationNotificationsMailer, type: :maile end describe 'assigned_conversation_new_message' do - let(:mail) { described_class.assigned_conversation_new_message(conversation, agent).deliver_now } + let(:message) { create(:message, conversation: conversation, account: account) } + let(:mail) { described_class.assigned_conversation_new_message(message, agent).deliver_now } it 'renders the subject' do - expect(mail.subject).to eq("#{agent.available_name}, New message in your assigned conversation [ID - #{conversation.display_id}].") + expect(mail.subject).to eq("#{agent.available_name}, New message in your assigned conversation [ID - #{message.conversation.display_id}].") end it 'renders the receiver email' do