TELCORE-355: report recorder open timeouts on record_session_error - #642
Open
minhtuan1407-telnyx wants to merge 1 commit into
Open
TELCORE-355: report recorder open timeouts on record_session_error#642minhtuan1407-telnyx wants to merge 1 commit into
minhtuan1407-telnyx wants to merge 1 commit into
Conversation
switch_core_file_open() collapsed every open failure into SWITCH_STATUS_GENERR, so a recorder-server timeout (rw_timeout expiry against rtmp://recorder...) was indistinguishable from a bad URL, a refused connection or a full disk. The concrete cause only ever reached the log line, which the Prometheus-based alerting in tel-core-alerts cannot read. mod_av now returns SWITCH_STATUS_TIMEOUT for an ETIMEDOUT open. switch_core_perform_file_open() propagates the file module's status unchanged through its failure path, so switch_ivr_async can report it as Record-Open-Timeout: true|false on record_session_error and mod_telnyx can count recorder timeouts separately (TELCORE-355). The status is the only channel that already survives a failed open: fh->params and the handle pool are destroyed on that path, and fh->event is caller-owned playback metadata. Every caller of switch_core_file_open() tests against SWITCH_STATUS_SUCCESS, so a second failure status is inert for them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
minhtuan1407-telnyx
force-pushed
the
linear-telcore-355
branch
from
August 13, 2026 15:06
e8f7f91 to
cccc052
Compare
dev-ryanc
approved these changes
Aug 13, 2026
dev-ryanc
left a comment
Collaborator
There was a problem hiding this comment.
PRBot automated review — no critical issues found. Approved based on: small mechanical change (18 additions, 8 deletions), error-path only — captures switch_core_file_open status and adds Record-Open-Timeout header to record_session_error event. Verified switch_core_perform_file_open passes module status through fail: path (line 396→469). No control flow change, no call-path impact on success path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
switch_core_file_open()collapsed every open failure intoSWITCH_STATUS_GENERR. When the recorder server timed out during INC-217 the only place the concrete cause appeared was the log line:The
record_session_errorevent that follows carries onlyRecord-File-Pathand a fixedRecord-Error: Error opening file, so a recorder-server timeout is indistinguishable from a bad URL, a refused connection or a full disk. All B2BUA alerting intel-core-alertsis Prometheus-expression based and cannot read logs, so the distinction has to reach the event to be alertable.Change
Two files, +18/-8.
mod_av:av_file_open()returnsSWITCH_STATUS_TIMEOUTinstead ofSWITCH_STATUS_GENERRwhenavio_open2()fails withAVERROR(ETIMEDOUT)— i.e.rw_timeoutexpiry.switch_ivr_async: capture the open status at the three record-open failure sites (single file, split in, split out) and addRecord-Open-Timeout: true|falsetorecord_session_error.Why the status and not a handle field or
handle->eventThe status is the only channel that already survives a failed open.
switch_core_perform_file_open()'sfail:path destroysfh->paramsand the handle memory pool (switch_core_file.c), so nothing allocated there lives past the return; and it propagates the file module'sstatusunchanged, so a distinct status arrives at the caller for free.fh->eventdoes survive, but it is caller-owned playback metadata merged intoPLAYBACK_START/PLAYBACK_STOP(switch_ivr_play_say.c:1685,:2087) and read bymod_http_cache, so a format module writing into it means either polluting caller data or handing ownership out of a failed open. A new field onswitch_file_handle_twould work but changes the layout of a struct that callers all over the tree stack-allocate — an avoidable rebuild hazard for a boolean that the return value can already carry.Blast radius
send_record_error_event()isstaticwith exactly three call sites, all updated.switch_core_file_open()call sites in the tree were checked: every one branches onSWITCH_STATUS_SUCCESS(==or!=) or propagates the status to a caller that does, so a second failure status is inert for them. The one test that asserts a specific failure status,avformat_test_read_err(test_avformat.c:208, expectsGENERR), is on the read path — this change is in the output-file branch ofav_file_open()and cannot reach it.Verification
USE_REGISTRY_IMAGE=1 make build-deb-package-on-dockerintelnyx_b2bua_builder(telnyx_b2bua-1.10.12-telv111.1.deb).An unreachable-but-routable target (connection refused) should report
Record-Open-Timeout: false, which is the other half of the check.Paired PR
Consumer: https://github.com/team-telnyx/mod_telnyx/pull/72 — counts the classified failures into
freeswitch_record_failure_total. That PR is independently deployable; without this one every open failure lands underreason="open-unknown".Linear: https://linear.app/telnyx/issue/TELCORE-355/add-b2bua-alerting-for-recorder-server-timeouts