fix(voice): lock webhook transitions and skip unknown terminates
- Wrap mark_outbound_accepted, accept_outbound_call, and handle_terminate in call.with_lock so they serialize against Whatsapp::CallService (which holds call.with_lock across the Meta API call). Without this, an ACCEPTED webhook arriving mid-terminate could overwrite the freshly-finalized terminal status back to in_progress and broadcast an erroneous outbound_accepted. - Drop build_missed_inbound_call. An outbound terminate landing in the tiny window between provider_service.initiate_call and Call.create! had no local row, so the fallback materialised an inbound row with the same provider_call_id and tripped the unique (provider, provider_call_id) index on the controller's create. The "out-of-order webhook" case it protected is rare in practice; trade that for eliminating the false-positive collision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
c5236ec67e
commit
dfdadc470d
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user