diff --git a/enterprise/app/services/whatsapp/incoming_call_service.rb b/enterprise/app/services/whatsapp/incoming_call_service.rb index 3977578d0..c5621950f 100644 --- a/enterprise/app/services/whatsapp/incoming_call_service.rb +++ b/enterprise/app/services/whatsapp/incoming_call_service.rb @@ -35,13 +35,18 @@ class Whatsapp::IncomingCallService end end + # with_lock + reload here serializes against Whatsapp::CallService (agent + # actions hold call.with_lock across the Meta API call), so a webhook racing + # with a terminate can't overwrite a freshly-finalized terminal status. def mark_outbound_accepted(call, payload) - return unless call.outgoing? - return if call.in_progress? || call.terminal? + call.with_lock do + next unless call.outgoing? + next if call.in_progress? || call.terminal? - started_at = Time.zone.at(payload[:timestamp].to_i) if payload[:timestamp].present? - update_call!(call, 'in_progress', started_at: started_at || Time.current) - broadcast(call, 'voice_call.outbound_accepted') + started_at = Time.zone.at(payload[:timestamp].to_i) if payload[:timestamp].present? + update_call!(call, 'in_progress', started_at: started_at || Time.current) + broadcast(call, 'voice_call.outbound_accepted') + end end def handle_connect(payload) @@ -81,42 +86,40 @@ class Whatsapp::IncomingCallService # Meta's SDP answer so the handshake completes during ringing; the call # stays in `ringing` until status=ACCEPTED arrives. def accept_outbound_call(call, payload) - return if call.in_progress? || call.terminal? + call.with_lock do + next if call.in_progress? || call.terminal? - # Pin setup:active so browsers don't renegotiate when Meta echoes actpass. - sdp_answer = payload.dig(:session, :sdp)&.gsub('a=setup:actpass', 'a=setup:active') - call.update!(meta: (call.meta || {}).merge('sdp_answer' => sdp_answer)) - broadcast(call, 'voice_call.outbound_connected', sdp_answer: sdp_answer) + # Pin setup:active so browsers don't renegotiate when Meta echoes actpass. + sdp_answer = payload.dig(:session, :sdp)&.gsub('a=setup:actpass', 'a=setup:active') + call.update!(meta: (call.meta || {}).merge('sdp_answer' => sdp_answer)) + broadcast(call, 'voice_call.outbound_connected', sdp_answer: sdp_answer) + end end def handle_terminate(payload) - # Webhooks can arrive out of order (terminate before connect under tunnel/network - # delays). Materialise a missed-call record so the contact's "called and hung up" - # still surfaces in the dashboard instead of being silently dropped. - call = Call.whatsapp.find_by(provider_call_id: payload[:id]) || build_missed_inbound_call(payload) - return unless call - # 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. - return if call.terminal? + 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" + return + end - duration = payload[:duration]&.to_i - status = answered?(call, duration) ? 'completed' : 'no_answer' - meta = (call.meta || {}).merge('ended_at' => Time.zone.now.to_i) - update_call!(call, status, duration_seconds: duration, end_reason: payload[:terminate_reason], meta: meta) - broadcast(call, 'voice_call.ended', status: call.display_status, duration_seconds: call.duration_seconds) - end + 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? - # No connect was ever processed (webhook reordering or never delivered) — build a - # bare Call+Message for the contact so the UI shows the missed-call bubble. - def build_missed_inbound_call(payload) - Voice::InboundCallBuilder.perform!( - inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id], - provider: :whatsapp, - extra_meta: { 'ice_servers' => Call.default_ice_servers } - ) - rescue ActiveRecord::RecordNotUnique - Call.whatsapp.find_by(provider_call_id: payload[:id]) + duration = payload[:duration]&.to_i + status = answered?(call, duration) ? 'completed' : 'no_answer' + meta = (call.meta || {}).merge('ended_at' => Time.zone.now.to_i) + update_call!(call, status, duration_seconds: duration, end_reason: payload[:terminate_reason], meta: meta) + broadcast(call, 'voice_call.ended', status: call.display_status, duration_seconds: call.duration_seconds) + end end # accepted_by_agent_id is the initiating agent on outbound calls, so it only signals "answered" for inbound. diff --git a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb index caf69d74c..5dacff6f2 100644 --- a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb @@ -114,6 +114,21 @@ describe Whatsapp::IncomingCallService do end end + describe 'terminate with no local row yet' do + it 'logs and skips instead of materialising an inbound missed-call row' do + allow(inbox.channel).to receive(:voice_enabled?).and_return(true) + allow(Rails.logger).to receive(:warn) + allow(ActionCable.server).to receive(:broadcast) + + params = call_payload(event: 'terminate', duration: 0, terminate_reason: 'no_answer') + + 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/) + expect(ActionCable.server).not_to have_received(:broadcast) + end + end + describe 'outbound connect with no local row yet' do it 'does not mint an inbound call when sdp_type is answer' do allow(inbox.channel).to receive(:voice_enabled?).and_return(true)