diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 431945896..b207bd2a4 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -1,6 +1,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob MAX_MESSAGE_LENGTH = 10_000 - retry_on ActiveStorage::FileNotFoundError, attempts: 3 + retry_on ActiveStorage::FileNotFoundError, attempts: 3, wait: 2.seconds + retry_on Faraday::BadRequestError, attempts: 3, wait: 2.seconds def perform(conversation, assistant) @conversation = conversation @@ -13,7 +14,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob generate_and_process_response end rescue StandardError => e - raise e if e.is_a?(ActiveStorage::FileNotFoundError) + raise e if e.is_a?(ActiveStorage::FileNotFoundError) || e.is_a?(Faraday::BadRequestError) handle_error(e) ensure @@ -50,9 +51,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob end def determine_role(message) - return 'system' if message.content.blank? - - message.message_type == 'incoming' ? 'user' : 'system' + message.message_type == 'incoming' ? 'user' : 'assistant' end def prepare_multimodal_message_content(message) diff --git a/enterprise/app/services/captain/open_ai_message_builder_service.rb b/enterprise/app/services/captain/open_ai_message_builder_service.rb index 3320ad537..43d2851c9 100644 --- a/enterprise/app/services/captain/open_ai_message_builder_service.rb +++ b/enterprise/app/services/captain/open_ai_message_builder_service.rb @@ -42,6 +42,7 @@ class Captain::OpenAiMessageBuilderService end def get_attachment_url(attachment) + return attachment.download_url if attachment.download_url.present? return attachment.external_url if attachment.external_url.present? attachment.file.attached? ? attachment.file_url : nil diff --git a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb index 92ccba553..e8faaf74a 100644 --- a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb +++ b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb @@ -4,10 +4,29 @@ module Enterprise::MessageTemplates::HookExecutionService return unless should_process_captain_response? return perform_handoff unless inbox.captain_active? - Captain::Conversation::ResponseBuilderJob.perform_later( - conversation, - conversation.inbox.captain_assistant - ) + schedule_captain_response + end + + private + + def schedule_captain_response + job_args = [conversation, conversation.inbox.captain_assistant] + + if message.attachments.blank? + Captain::Conversation::ResponseBuilderJob.perform_later(*job_args) + else + wait_time = calculate_attachment_wait_time + Captain::Conversation::ResponseBuilderJob.set(wait: wait_time).perform_later(*job_args) + end + end + + def calculate_attachment_wait_time + attachment_count = message.attachments.size + base_wait = 1.second + + # Wait longer for more attachments or larger files + additional_wait = [attachment_count * 1, 4].min.seconds + base_wait + additional_wait end def should_process_captain_response? diff --git a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb index ca8d4a6c0..c21205d52 100644 --- a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb @@ -56,4 +56,116 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do end end end + + describe 'retry mechanisms for image processing' do + let(:conversation) { create(:conversation, inbox: inbox, account: account) } + let(:mock_llm_chat_service) { instance_double(Captain::Llm::AssistantChatService) } + let(:mock_message_builder) { instance_double(Captain::OpenAiMessageBuilderService) } + + before do + create(:message, conversation: conversation, content: 'Hello with image', message_type: :incoming) + allow(Captain::Llm::AssistantChatService).to receive(:new).and_return(mock_llm_chat_service) + allow(Captain::OpenAiMessageBuilderService).to receive(:new).with(message: anything).and_return(mock_message_builder) + allow(mock_message_builder).to receive(:generate_content).and_return('Hello with image') + allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'Test response' }) + end + + context 'when ActiveStorage::FileNotFoundError occurs' do + it 'handles file errors and triggers handoff' do + allow(mock_message_builder).to receive(:generate_content) + .and_raise(ActiveStorage::FileNotFoundError, 'Image file not found') + + # For retryable errors, the job should handle them and proceed with handoff + described_class.perform_now(conversation, assistant) + + # Verify handoff occurred due to repeated failures + expect(conversation.reload.status).to eq('open') + end + + it 'succeeds when no error occurs' do + # Don't raise any error, should succeed normally + allow(mock_message_builder).to receive(:generate_content) + .and_return('Image content processed successfully') + + described_class.perform_now(conversation, assistant) + + expect(conversation.messages.outgoing.count).to eq(1) + expect(conversation.messages.outgoing.last.content).to eq('Test response') + end + end + + context 'when Faraday::BadRequestError occurs' do + it 'handles API errors and triggers handoff' do + allow(mock_llm_chat_service).to receive(:generate_response) + .and_raise(Faraday::BadRequestError, 'Bad request to image service') + + described_class.perform_now(conversation, assistant) + expect(conversation.reload.status).to eq('open') + end + + it 'succeeds when no error occurs' do + # Don't raise any error, should succeed normally + allow(mock_llm_chat_service).to receive(:generate_response) + .and_return({ 'response' => 'Response after retry' }) + + described_class.perform_now(conversation, assistant) + + expect(conversation.messages.outgoing.last.content).to eq('Response after retry') + end + end + + context 'when image processing fails permanently' do + before do + allow(mock_message_builder).to receive(:generate_content) + .and_raise(ActiveStorage::FileNotFoundError, 'Image permanently unavailable') + end + + it 'triggers handoff after max retries' do + # Since perform_now re-raises retryable errors, simulate the final failure after retries + allow(mock_message_builder).to receive(:generate_content) + .and_raise(StandardError, 'Max retries exceeded') + + expect(ChatwootExceptionTracker).to receive(:new).and_call_original + + described_class.perform_now(conversation, assistant) + + expect(conversation.reload.status).to eq('open') + end + end + + context 'when non-retryable error occurs' do + let(:standard_error) { StandardError.new('Generic error') } + + before do + allow(mock_llm_chat_service).to receive(:generate_response).and_raise(standard_error) + end + + it 'handles error and triggers handoff' do + expect(ChatwootExceptionTracker).to receive(:new) + .with(standard_error, account: account) + .and_call_original + + described_class.perform_now(conversation, assistant) + + expect(conversation.reload.status).to eq('open') + end + + it 'ensures Current.executed_by is reset' do + expect(Current).to receive(:executed_by=).with(assistant) + expect(Current).to receive(:executed_by=).with(nil) + + described_class.perform_now(conversation, assistant) + end + end + end + + describe 'job configuration' do + it 'has retry_on configuration for retryable errors' do + expect(described_class).to respond_to(:retry_on) + end + + it 'defines MAX_MESSAGE_LENGTH constant' do + expect(described_class::MAX_MESSAGE_LENGTH).to eq(10_000) + end + end end diff --git a/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb index 13c29f756..76e91ae7a 100644 --- a/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb +++ b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb @@ -207,6 +207,7 @@ RSpec.describe Captain::OpenAiMessageBuilderService do attachment.update(external_url: nil) allow(attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: true)) allow(attachment).to receive(:file_url).and_return('https://local.com/file.jpg') + allow(attachment).to receive(:download_url).and_return('') end it 'returns file_url' do