diff --git a/app/javascript/dashboard/composables/useCallSession.js b/app/javascript/dashboard/composables/useCallSession.js index 354f3a7b3..275a6337d 100644 --- a/app/javascript/dashboard/composables/useCallSession.js +++ b/app/javascript/dashboard/composables/useCallSession.js @@ -161,6 +161,7 @@ const buildCallActions = ({ callsStore, whatsappSession, t }) => { return { conferenceSid: joinResponse?.conference_sid }; } catch (error) { useAlert(error?.response?.data?.error || t('CONTACT_PANEL.CALL_FAILED')); + // 409 = the call already ended before accept landed (e.g. caller hung up mid-ring). if (error?.response?.status === 409) { TwilioVoiceClient.endClientCall(); markDismissed(callSid); diff --git a/app/javascript/dashboard/helper/voice.js b/app/javascript/dashboard/helper/voice.js index 224dce88a..4f5ad25fe 100644 --- a/app/javascript/dashboard/helper/voice.js +++ b/app/javascript/dashboard/helper/voice.js @@ -1,4 +1,7 @@ -import { CONTENT_TYPES } from 'dashboard/components-next/message/constants'; +import { + CONTENT_TYPES, + VOICE_CALL_STATUS, +} from 'dashboard/components-next/message/constants'; import { MESSAGE_TYPE } from 'shared/constants/messages'; import { useCallsStore } from 'dashboard/stores/calls'; import types from 'dashboard/store/mutation-types'; @@ -101,6 +104,7 @@ export function handleVoiceCallCreated( callSid, callId, provider, + status, callDirection, conversationId, inboxId, @@ -108,6 +112,11 @@ export function handleVoiceCallCreated( senderId, } = extractCallData(message); + // A voice_call message can be created already terminal when the caller hangs + // up before connect. Only ring while the call is actually ringing; mirrors the + // guard in seedCallsFromHydratedMessages. + if (status !== VOICE_CALL_STATUS.RINGING) return; + if ( !shouldShowCall({ callDirection, diff --git a/config/locales/en.yml b/config/locales/en.yml index 22d3630af..d27c5d962 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -134,6 +134,7 @@ en: not_supported: 'Reauthorization is not supported for this type of WhatsApp channel.' calls: not_enabled: 'Calling is not enabled for this inbox' + already_ended: 'This call has already ended' no_recording: 'No recording file provided' no_message: 'Call has no associated message' sdp_offer_required: 'sdp_offer is required' diff --git a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb index 0301d428b..28dd378da 100644 --- a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb @@ -13,6 +13,7 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro Voice::CallErrors::AlreadyAccepted, Voice::CallErrors::CallFailed, with: :render_call_error + rescue_from Voice::CallErrors::CallAlreadyEnded, with: :render_call_ended rescue_from Voice::CallErrors::NoCallPermission, with: :render_permission_request def show; end @@ -190,4 +191,9 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro def render_call_error(error) render_could_not_create_error(error.message) end + + # 409 (not 422) so the FE can tell "already ended" from a generic failure and dismiss the ringing UI. + def render_call_ended + render json: { error: I18n.t('errors.whatsapp.calls.already_ended') }, status: :conflict + end end diff --git a/enterprise/app/services/whatsapp/call_service.rb b/enterprise/app/services/whatsapp/call_service.rb index 362af33cb..bd0dd6ae5 100644 --- a/enterprise/app/services/whatsapp/call_service.rb +++ b/enterprise/app/services/whatsapp/call_service.rb @@ -48,9 +48,10 @@ class Whatsapp::CallService private def transition_to_in_progress! - # Order matters: in_progress and terminal both make ringing? false, so we have to - # branch on in_progress? first to surface the distinct AlreadyAccepted state. + # in_progress and terminal both make ringing? false; branch in order to surface the + # distinct AlreadyAccepted / CallAlreadyEnded states (caller can hang up mid-ring). raise Voice::CallErrors::AlreadyAccepted, 'Call already accepted by another agent' if call.in_progress? + raise Voice::CallErrors::CallAlreadyEnded, 'Call already ended' if call.terminal? raise Voice::CallErrors::NotRinging, 'Call is not in ringing state' unless call.ringing? forward_answer_to_meta! diff --git a/enterprise/app/services/whatsapp/incoming_call_service.rb b/enterprise/app/services/whatsapp/incoming_call_service.rb index f6185050d..11b35d95a 100644 --- a/enterprise/app/services/whatsapp/incoming_call_service.rb +++ b/enterprise/app/services/whatsapp/incoming_call_service.rb @@ -1,6 +1,9 @@ class Whatsapp::IncomingCallService pattr_initialize [:inbox!, :params!] + # Lifespan of a terminate-before-connect tombstone; the paired connect arrives within ~1s. + TERMINATE_TOMBSTONE_TTL = 60 + def perform return unless inbox.channel.voice_enabled? @@ -79,16 +82,32 @@ class Whatsapp::IncomingCallService end sdp_offer = payload.dig(:session, :sdp) + call = build_inbound_call(payload, sdp_offer) + + return if call.terminal? # terminated before pickup; no ringing widget to surface + + update_conversation(call) + broadcast_incoming(call, sdp_offer) + end + + # If a terminate already arrived (caller hung up before pickup), finalize it in the + # SAME transaction as the build so the message's after_create_commit fires (at outer + # commit) already terminal, never `ringing` — agents aren't rung for a dead call. + def build_inbound_call(payload, sdp_offer) + ActiveRecord::Base.transaction do + call = Voice::InboundCallBuilder.perform!(inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id], + provider: :whatsapp, extra_meta: inbound_extra_meta(payload, sdp_offer)) + tombstone = consume_terminate_tombstone(payload[:id]) + finalize_terminate(call, tombstone['duration'], tombstone['terminate_reason']) if tombstone + call + end + end + + def inbound_extra_meta(payload, sdp_offer) extra_meta = { 'sdp_offer' => sdp_offer, 'ice_servers' => Call.default_ice_servers } name = caller_profile_name(payload) extra_meta['contact_name'] = name if name.present? - - call = Voice::InboundCallBuilder.perform!( - inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id], - provider: :whatsapp, extra_meta: extra_meta - ) - update_conversation(call) - broadcast_incoming(call, sdp_offer) + extra_meta end # Match strictly on wa_id (== calls[].from): in a batched payload missing this @@ -122,23 +141,24 @@ class Whatsapp::IncomingCallService def handle_terminate(payload) call = Call.whatsapp.find_by(provider_call_id: payload[:id]) if call.nil? - # No row yet means either an out-of-order terminate (rare in practice — Meta - # delivery is FIFO) or, more dangerously, an outbound terminate landing in - # the window between the controller's Meta API call and Call.create!. - # Materialising as inbound here would collide with the unique - # (provider, provider_call_id) index. Skip; controller commits seal it. - Rails.logger.warn "[WHATSAPP CALL] Terminate for unknown call #{payload[:id]}; skipping" + # Terminate overtook its connect (Meta isn't strictly ordered); tombstone it for the + # connect handler to consume. An outbound tombstone just expires unused. + record_terminate_tombstone(payload) return end + finalize_terminate(call, payload[:duration], payload[:terminate_reason]) + end + + def finalize_terminate(call, duration, reason) + duration = duration&.to_i + reason = reason.to_s call.with_lock do # Webhook retries can re-deliver terminate after we've already finalized the # call; don't recompute status or a duration=0 retry can flip a completed # short call back to no_answer. next if call.terminal? - duration = payload[:duration]&.to_i - reason = payload[:terminate_reason].to_s status = derive_terminate_status(call, duration, reason) meta = (call.meta || {}).merge('ended_at' => Time.zone.now.to_i) update_call!(call, status, duration_seconds: duration, end_reason: reason, meta: meta) @@ -146,6 +166,28 @@ class Whatsapp::IncomingCallService end end + def record_terminate_tombstone(payload) + Redis::Alfred.setex( + terminate_tombstone_key(payload[:id]), + { 'duration' => payload[:duration], 'terminate_reason' => payload[:terminate_reason] }.to_json, + TERMINATE_TOMBSTONE_TTL + ) + Rails.logger.info "[WHATSAPP CALL] Terminate before connect for #{payload[:id]}; tombstoned" + end + + def consume_terminate_tombstone(provider_call_id) + key = terminate_tombstone_key(provider_call_id) + raw = Redis::Alfred.get(key) + return nil if raw.blank? + + Redis::Alfred.delete(key) + JSON.parse(raw) + end + + def terminate_tombstone_key(provider_call_id) + format(Redis::Alfred::WHATSAPP_CALL_TERMINATE_TOMBSTONE, call_id: provider_call_id) + end + # Provider-reported failures trump the answered/no_answer heuristic. An # in_progress call that Meta later terminates with a failure reason would # otherwise be recorded as 'completed' purely because it had been accepted. diff --git a/enterprise/lib/voice/call_errors.rb b/enterprise/lib/voice/call_errors.rb index 6b53ddbdc..e45edf0c0 100644 --- a/enterprise/lib/voice/call_errors.rb +++ b/enterprise/lib/voice/call_errors.rb @@ -8,4 +8,5 @@ module Voice::CallErrors class CallFailed < StandardError; end class NotRinging < StandardError; end class AlreadyAccepted < StandardError; end + class CallAlreadyEnded < StandardError; end end diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index bc7fb551e..6d503df22 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -58,6 +58,8 @@ module Redis::RedisKeys # Check if a message create with same source-id is in progress? MESSAGE_SOURCE_KEY = 'MESSAGE_SOURCE_KEY::%s'.freeze OPENAI_CONVERSATION_KEY = 'OPEN_AI_CONVERSATION_KEY::V1::%s::%d::%d'.freeze + # Bridges a WhatsApp call `terminate` that overtook its `connect` so the later connect can finalize it. + WHATSAPP_CALL_TERMINATE_TOMBSTONE = 'WHATSAPP_CALL_TERMINATE_TOMBSTONE::%s'.freeze ## Sempahores / Locks # We don't want to process messages from the same sender concurrently to prevent creating double conversations diff --git a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb index 500255983..e0027e273 100644 --- a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb @@ -58,6 +58,15 @@ RSpec.describe 'WhatsApp Calls API', type: :request do expect(response).to have_http_status(:unprocessable_entity) end + + it 'returns 409 when the call has already ended (caller hung up mid-ring)' do + call.update!(status: 'no_answer') + + post "/api/v1/accounts/#{account.id}/whatsapp_calls/#{call.id}/accept", + params: { sdp_answer: 'sdp_answer' }, headers: agent.create_new_auth_token + + expect(response).to have_http_status(:conflict) + end end describe 'POST /api/v1/accounts/:account_id/whatsapp_calls/:id/reject' do diff --git a/spec/enterprise/services/whatsapp/call_service_spec.rb b/spec/enterprise/services/whatsapp/call_service_spec.rb index 926771a65..4620ca588 100644 --- a/spec/enterprise/services/whatsapp/call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/call_service_spec.rb @@ -57,11 +57,11 @@ describe Whatsapp::CallService do .to raise_error(StandardError) { |error| expect(error.class.name).to eq('Voice::CallErrors::AlreadyAccepted') } end - it 'raises NotRinging when the call has reached a terminal state' do + it 'raises CallAlreadyEnded when the call has reached a terminal state' do call.update!(status: 'completed') expect { described_class.new(call: call, agent: agent, sdp_answer: sdp_answer).accept } - .to raise_error(StandardError) { |error| expect(error.class.name).to eq('Voice::CallErrors::NotRinging') } + .to raise_error(StandardError) { |error| expect(error.class.name).to eq('Voice::CallErrors::CallAlreadyEnded') } end it 'raises CallFailed when sdp_answer is missing' do diff --git a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb index 4651b5f13..a3c5246d2 100644 --- a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb @@ -149,17 +149,39 @@ describe Whatsapp::IncomingCallService do end describe 'terminate with no local row yet' do - it 'logs and skips instead of materialising an inbound missed-call row' do - allow(Rails.logger).to receive(:warn) + # Unique per example: the 60s tombstone isn't rolled back between specs. + let(:tombstone_call_id) { "wacid.#{SecureRandom.hex(6)}" } + + after { Redis::Alfred.delete(format(Redis::Alfred::WHATSAPP_CALL_TERMINATE_TOMBSTONE, call_id: tombstone_call_id)) } + + it 'tombstones the terminate instead of materialising an inbound missed-call row' do allow(ActionCable.server).to receive(:broadcast) params = call_payload(event: 'terminate', duration: 0, terminate_reason: 'no_answer') + params[:calls][0][:id] = tombstone_call_id expect { described_class.new(inbox: inbox, params: params).perform } .not_to change(Call, :count) - expect(Rails.logger).to have_received(:warn).with(/Terminate for unknown call/) + key = format(Redis::Alfred::WHATSAPP_CALL_TERMINATE_TOMBSTONE, call_id: tombstone_call_id) + expect(Redis::Alfred.get(key)).to be_present expect(ActionCable.server).not_to have_received(:broadcast) end + + it 'finalizes the call as no_answer when the connect arrives after the tombstone' do + allow(ActionCable.server).to receive(:broadcast) + + terminate = call_payload(event: 'terminate', duration: 0, terminate_reason: 'no_answer') + terminate[:calls][0][:id] = tombstone_call_id + described_class.new(inbox: inbox, params: terminate).perform + + connect = call_payload(event: 'connect', session: { sdp: 'v=0', sdp_type: 'offer' }) + connect[:calls][0][:id] = tombstone_call_id + expect { described_class.new(inbox: inbox, params: connect).perform } + .to change(Call, :count).by(1) + expect(Call.find_by(provider_call_id: tombstone_call_id).status).to eq('no_answer') + expect(ActionCable.server).to have_received(:broadcast) + .with(anything, hash_including(event: 'voice_call.ended')).at_least(:once) + end end describe 'outbound connect with no local row yet' do