DataChannel: Propagate SCTP transport errors to the channels
When the transport is terminated, if an error has occured, it will
be propagated to the channels.
When such errors can happen at the SCTP level (e.g. out of resources),
RTCError may contain an error code matching the definition at
https://www.iana.org/assignments/sctp-parameters/sctp-parameters.xhtml#sctp-parameters-24
If the m= line is rejected or removed from SDP, an error will again be sent
to the data channels, signaling their unexpected transition to closed.
Bug: webrtc:12904
Change-Id: Iea3d8aba0a57bbedb5d03f0fb6f7aba292e92fe8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223541
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34386}
diff --git a/api/transport/data_channel_transport_interface.h b/api/transport/data_channel_transport_interface.h
index 550faba..2b2f5d2 100644
--- a/api/transport/data_channel_transport_interface.h
+++ b/api/transport/data_channel_transport_interface.h
@@ -88,7 +88,7 @@
// Callback issued when the data channel becomes unusable (closed).
// TODO(https://crbug.com/webrtc/10360): Make pure virtual when all
// consumers updated.
- virtual void OnTransportClosed() {}
+ virtual void OnTransportClosed(RTCError error) {}
};
// Transport for data channels.
diff --git a/media/sctp/dcsctp_transport.cc b/media/sctp/dcsctp_transport.cc
index 90fb0e8..d913f0c 100644
--- a/media/sctp/dcsctp_transport.cc
+++ b/media/sctp/dcsctp_transport.cc
@@ -73,6 +73,28 @@
return absl::nullopt;
}
+absl::optional<cricket::SctpErrorCauseCode> ToErrorCauseCode(
+ dcsctp::ErrorKind error) {
+ switch (error) {
+ case dcsctp::ErrorKind::kParseFailed:
+ return cricket::SctpErrorCauseCode::kUnrecognizedParameters;
+ case dcsctp::ErrorKind::kPeerReported:
+ return cricket::SctpErrorCauseCode::kUserInitiatedAbort;
+ case dcsctp::ErrorKind::kWrongSequence:
+ case dcsctp::ErrorKind::kProtocolViolation:
+ return cricket::SctpErrorCauseCode::kProtocolViolation;
+ case dcsctp::ErrorKind::kResourceExhaustion:
+ return cricket::SctpErrorCauseCode::kOutOfResource;
+ case dcsctp::ErrorKind::kTooManyRetries:
+ case dcsctp::ErrorKind::kUnsupportedOperation:
+ case dcsctp::ErrorKind::kNoError:
+ case dcsctp::ErrorKind::kNotConnected:
+ // No SCTP error cause code matches those
+ break;
+ }
+ return absl::nullopt;
+}
+
bool IsEmptyPPID(dcsctp::PPID ppid) {
WebrtcPPID webrtc_ppid = static_cast<WebrtcPPID>(ppid.value());
return webrtc_ppid == WebrtcPPID::kStringEmpty ||
@@ -413,6 +435,14 @@
<< "->OnAborted(error=" << dcsctp::ToString(error)
<< ", message=" << message << ").";
ready_to_send_data_ = false;
+ RTCError rtc_error(RTCErrorType::OPERATION_ERROR_WITH_DATA,
+ std::string(message));
+ rtc_error.set_error_detail(RTCErrorDetailType::SCTP_FAILURE);
+ auto code = ToErrorCauseCode(error);
+ if (code.has_value()) {
+ rtc_error.set_sctp_cause_code(static_cast<uint16_t>(*code));
+ }
+ SignalClosedAbruptly(rtc_error);
}
void DcSctpTransport::OnConnected() {
@@ -520,7 +550,7 @@
void DcSctpTransport::OnTransportClosed(
rtc::PacketTransportInternal* transport) {
RTC_LOG(LS_VERBOSE) << debug_name_ << "->OnTransportClosed().";
- SignalClosedAbruptly();
+ SignalClosedAbruptly({});
}
void DcSctpTransport::MaybeConnectSocket() {
diff --git a/media/sctp/sctp_transport_internal.h b/media/sctp/sctp_transport_internal.h
index 96c35ff..b132716 100644
--- a/media/sctp/sctp_transport_internal.h
+++ b/media/sctp/sctp_transport_internal.h
@@ -53,6 +53,24 @@
// usrsctp.h)
const int kSctpDefaultPort = 5000;
+// Error cause codes defined at
+// https://www.iana.org/assignments/sctp-parameters/sctp-parameters.xhtml#sctp-parameters-24
+enum class SctpErrorCauseCode : uint16_t {
+ kInvalidStreamIdentifier = 1,
+ kMissingMandatoryParameter = 2,
+ kStaleCookieError = 3,
+ kOutOfResource = 4,
+ kUnresolvableAddress = 5,
+ kUnrecognizedChunkType = 6,
+ kInvalidMandatoryParameter = 7,
+ kUnrecognizedParameters = 8,
+ kNoUserData = 9,
+ kCookieReceivedWhileShuttingDown = 10,
+ kRestartWithNewAddresses = 11,
+ kUserInitiatedAbort = 12,
+ kProtocolViolation = 13,
+};
+
// Abstract SctpTransport interface for use internally (by PeerConnection etc.).
// Exists to allow mock/fake SctpTransports to be created.
class SctpTransportInternal {
@@ -137,8 +155,8 @@
// and outgoing streams reset).
sigslot::signal1<int> SignalClosingProcedureComplete;
// Fired when the underlying DTLS transport has closed due to an error
- // or an incoming DTLS disconnect.
- sigslot::signal0<> SignalClosedAbruptly;
+ // or an incoming DTLS disconnect or SCTP transport errors.
+ sigslot::signal1<webrtc::RTCError> SignalClosedAbruptly;
// Helper for debugging.
virtual void set_debug_name_for_testing(const char* debug_name) = 0;
diff --git a/media/sctp/usrsctp_transport.cc b/media/sctp/usrsctp_transport.cc
index d43c017..7824a72 100644
--- a/media/sctp/usrsctp_transport.cc
+++ b/media/sctp/usrsctp_transport.cc
@@ -53,6 +53,7 @@
#include "rtc_base/thread_annotations.h"
#include "rtc_base/trace_event.h"
+namespace cricket {
namespace {
// The biggest SCTP packet. Starting from a 'safe' wire MTU value of 1280,
@@ -236,9 +237,39 @@
}
return spa;
}
-} // namespace
-namespace cricket {
+std::string SctpErrorCauseCodeToString(SctpErrorCauseCode code) {
+ switch (code) {
+ case SctpErrorCauseCode::kInvalidStreamIdentifier:
+ return "Invalid Stream Identifier";
+ case SctpErrorCauseCode::kMissingMandatoryParameter:
+ return "Missing Mandatory Parameter";
+ case SctpErrorCauseCode::kStaleCookieError:
+ return "Stale Cookie Error";
+ case SctpErrorCauseCode::kOutOfResource:
+ return "Out of Resource";
+ case SctpErrorCauseCode::kUnresolvableAddress:
+ return "Unresolvable Address";
+ case SctpErrorCauseCode::kUnrecognizedChunkType:
+ return "Unrecognized Chunk Type";
+ case SctpErrorCauseCode::kInvalidMandatoryParameter:
+ return "Invalid Mandatory Parameter";
+ case SctpErrorCauseCode::kUnrecognizedParameters:
+ return "Unrecognized Parameters";
+ case SctpErrorCauseCode::kNoUserData:
+ return "No User Data";
+ case SctpErrorCauseCode::kCookieReceivedWhileShuttingDown:
+ return "Cookie Received Whilte Shutting Down";
+ case SctpErrorCauseCode::kRestartWithNewAddresses:
+ return "Restart With New Addresses";
+ case SctpErrorCauseCode::kUserInitiatedAbort:
+ return "User Initiated Abort";
+ case SctpErrorCauseCode::kProtocolViolation:
+ return "Protocol Violation";
+ }
+ return "Unknown error";
+}
+} // namespace
// Maps SCTP transport ID to UsrsctpTransport object, necessary in send
// threshold callback and outgoing packet callback. It also provides a facility
@@ -1211,7 +1242,11 @@
}
void UsrsctpTransport::OnClosed(rtc::PacketTransportInternal* transport) {
- SignalClosedAbruptly();
+ webrtc::RTCError error =
+ webrtc::RTCError(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA,
+ "Transport channel closed");
+ error.set_error_detail(webrtc::RTCErrorDetailType::SCTP_FAILURE);
+ SignalClosedAbruptly(error);
}
void UsrsctpTransport::OnSendThresholdCallback() {
@@ -1497,9 +1532,17 @@
// came up, send any queued resets.
SendQueuedStreamResets();
break;
- case SCTP_COMM_LOST:
+ case SCTP_COMM_LOST: {
RTC_LOG(LS_INFO) << "Association change SCTP_COMM_LOST";
+ webrtc::RTCError error = webrtc::RTCError(
+ webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA,
+ SctpErrorCauseCodeToString(
+ static_cast<SctpErrorCauseCode>(change.sac_error)));
+ error.set_error_detail(webrtc::RTCErrorDetailType::SCTP_FAILURE);
+ error.set_sctp_cause_code(change.sac_error);
+ SignalClosedAbruptly(error);
break;
+ }
case SCTP_RESTART:
RTC_LOG(LS_INFO) << "Association change SCTP_RESTART";
break;
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 59b0558..460462e 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -368,6 +368,7 @@
"../rtc_base/system:file_wrapper",
"../rtc_base/system:no_unique_address",
"../rtc_base/system:rtc_export",
+ "../rtc_base/system:unused",
"../rtc_base/task_utils:pending_task_safety_flag",
"../rtc_base/task_utils:to_queued_task",
"../rtc_base/third_party/base64",
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index d8e6b39..7e9724d 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -162,13 +162,13 @@
}));
}
-void DataChannelController::OnTransportClosed() {
+void DataChannelController::OnTransportClosed(RTCError error) {
RTC_DCHECK_RUN_ON(network_thread());
signaling_thread()->PostTask(
- ToQueuedTask([self = weak_factory_.GetWeakPtr()] {
+ ToQueuedTask([self = weak_factory_.GetWeakPtr(), error] {
if (self) {
RTC_DCHECK_RUN_ON(self->signaling_thread());
- self->OnTransportChannelClosed();
+ self->OnTransportChannelClosed(error);
}
}));
}
@@ -351,14 +351,14 @@
}
}
-void DataChannelController::OnTransportChannelClosed() {
+void DataChannelController::OnTransportChannelClosed(RTCError error) {
RTC_DCHECK_RUN_ON(signaling_thread());
// Use a temporary copy of the SCTP DataChannel list because the
// DataChannel may callback to us and try to modify the list.
std::vector<rtc::scoped_refptr<SctpDataChannel>> temp_sctp_dcs;
temp_sctp_dcs.swap(sctp_data_channels_);
for (const auto& channel : temp_sctp_dcs) {
- channel->OnTransportChannelClosed();
+ channel->OnTransportChannelClosed(error);
}
}
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h
index 05fcff0..7b1ff26 100644
--- a/pc/data_channel_controller.h
+++ b/pc/data_channel_controller.h
@@ -70,7 +70,7 @@
void OnChannelClosing(int channel_id) override;
void OnChannelClosed(int channel_id) override;
void OnReadyToSend() override;
- void OnTransportClosed() override;
+ void OnTransportClosed(RTCError error) override;
// Called from PeerConnection::SetupDataChannelTransport_n
void SetupDataChannelTransport_n();
@@ -111,7 +111,7 @@
return SignalSctpDataChannelCreated_;
}
// Called when the transport for the data channels is closed or destroyed.
- void OnTransportChannelClosed();
+ void OnTransportChannelClosed(RTCError error);
void OnSctpDataChannelClosed(SctpDataChannel* channel);
diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc
index 98c44f2..770892c 100644
--- a/pc/data_channel_unittest.cc
+++ b/pc/data_channel_unittest.cc
@@ -13,6 +13,7 @@
#include <memory>
#include <vector>
+#include "media/sctp/sctp_transport_internal.h"
#include "pc/sctp_data_channel.h"
#include "pc/sctp_utils.h"
#include "pc/test/fake_data_channel_provider.h"
@@ -635,7 +636,9 @@
// Tell the data channel that its transport is being destroyed.
// It should then stop using the transport (allowing us to delete it) and
// transition to the "closed" state.
- webrtc_data_channel_->OnTransportChannelClosed();
+ webrtc::RTCError error(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA, "");
+ error.set_error_detail(webrtc::RTCErrorDetailType::SCTP_FAILURE);
+ webrtc_data_channel_->OnTransportChannelClosed(error);
provider_.reset(nullptr);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state(), kDefaultTimeout);
@@ -646,6 +649,31 @@
webrtc_data_channel_->error().error_detail());
}
+TEST_F(SctpDataChannelTest, TransportGotErrorCode) {
+ SetChannelReady();
+
+ // Tell the data channel that its transport is being destroyed with an
+ // error code.
+ // It should then report that error code.
+ webrtc::RTCError error(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA,
+ "Transport channel closed");
+ error.set_error_detail(webrtc::RTCErrorDetailType::SCTP_FAILURE);
+ error.set_sctp_cause_code(
+ static_cast<uint16_t>(cricket::SctpErrorCauseCode::kProtocolViolation));
+ webrtc_data_channel_->OnTransportChannelClosed(error);
+ provider_.reset(nullptr);
+ EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
+ webrtc_data_channel_->state(), kDefaultTimeout);
+ EXPECT_FALSE(webrtc_data_channel_->error().ok());
+ EXPECT_EQ(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA,
+ webrtc_data_channel_->error().type());
+ EXPECT_EQ(webrtc::RTCErrorDetailType::SCTP_FAILURE,
+ webrtc_data_channel_->error().error_detail());
+ EXPECT_EQ(
+ static_cast<uint16_t>(cricket::SctpErrorCauseCode::kProtocolViolation),
+ webrtc_data_channel_->error().sctp_cause_code());
+}
+
class SctpSidAllocatorTest : public ::testing::Test {
protected:
SctpSidAllocator allocator_;
diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index 359cc79..0e4ef7d 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -22,6 +22,7 @@
#include "rtc_base/location.h"
#include "rtc_base/logging.h"
#include "rtc_base/ref_counted_object.h"
+#include "rtc_base/system/unused.h"
#include "rtc_base/task_utils/to_queued_task.h"
#include "rtc_base/thread.h"
@@ -178,6 +179,7 @@
observer_(nullptr),
provider_(provider) {
RTC_DCHECK_RUN_ON(signaling_thread_);
+ RTC_UNUSED(network_thread_);
}
bool SctpDataChannel::Init() {
@@ -381,13 +383,11 @@
}
}
-void SctpDataChannel::OnTransportChannelClosed() {
- // The SctpTransport is unusable (for example, because the SCTP m= section
- // was rejected, or because the DTLS transport closed), so we need to close
- // abruptly.
- RTCError error = RTCError(RTCErrorType::OPERATION_ERROR_WITH_DATA,
- "Transport channel closed");
- error.set_error_detail(RTCErrorDetailType::SCTP_FAILURE);
+void SctpDataChannel::OnTransportChannelClosed(RTCError error) {
+ // The SctpTransport is unusable, which could come from multiplie reasons:
+ // - the SCTP m= section was rejected
+ // - the DTLS transport is closed
+ // - the SCTP transport is closed
CloseAbruptlyWithError(std::move(error));
}
diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h
index 1d7a3c7..b0df487 100644
--- a/pc/sctp_data_channel.h
+++ b/pc/sctp_data_channel.h
@@ -177,8 +177,6 @@
void CloseAbruptlyWithError(RTCError error);
// Specializations of CloseAbruptlyWithError
void CloseAbruptlyWithDataChannelFailure(const std::string& message);
- void CloseAbruptlyWithSctpCauseCode(const std::string& message,
- uint16_t cause_code);
// Slots for provider to connect signals to.
//
@@ -209,7 +207,7 @@
// Called when the transport channel is unusable.
// This method makes sure the DataChannel is disconnected and changes state
// to kClosed.
- void OnTransportChannelClosed();
+ void OnTransportChannelClosed(RTCError error);
DataChannelStats GetStats() const;
diff --git a/pc/sctp_data_channel_transport.cc b/pc/sctp_data_channel_transport.cc
index bb81156..f01f86e 100644
--- a/pc/sctp_data_channel_transport.cc
+++ b/pc/sctp_data_channel_transport.cc
@@ -102,9 +102,9 @@
}
}
-void SctpDataChannelTransport::OnClosedAbruptly() {
+void SctpDataChannelTransport::OnClosedAbruptly(RTCError error) {
if (sink_) {
- sink_->OnTransportClosed();
+ sink_->OnTransportClosed(error);
}
}
diff --git a/pc/sctp_data_channel_transport.h b/pc/sctp_data_channel_transport.h
index 30818ab..4b89205e 100644
--- a/pc/sctp_data_channel_transport.h
+++ b/pc/sctp_data_channel_transport.h
@@ -41,7 +41,7 @@
const rtc::CopyOnWriteBuffer& buffer);
void OnClosingProcedureStartedRemotely(int channel_id);
void OnClosingProcedureComplete(int channel_id);
- void OnClosedAbruptly();
+ void OnClosedAbruptly(RTCError error);
cricket::SctpTransportInternal* const sctp_transport_;
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 2bfb61a..533bd84 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -3395,8 +3395,14 @@
const cricket::ContentInfo& content,
const cricket::ContentGroup* bundle_group) {
if (content.rejected) {
- RTC_LOG(LS_INFO) << "Rejected data channel, mid=" << content.mid();
- DestroyDataChannelTransport();
+ RTC_LOG(LS_INFO) << "Rejected data channel transport with mid="
+ << content.mid();
+
+ rtc::StringBuilder sb;
+ sb << "Rejected data channel transport with mid=" << content.mid();
+ RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release());
+ error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
+ DestroyDataChannelTransport(error);
} else {
if (!data_channel_controller()->data_channel_transport()) {
RTC_LOG(LS_INFO) << "Creating data channel, mid=" << content.mid();
@@ -4369,8 +4375,18 @@
}
const cricket::ContentInfo* data_info = cricket::GetFirstDataContent(desc);
- if (!data_info || data_info->rejected) {
- DestroyDataChannelTransport();
+ if (!data_info) {
+ RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA,
+ "No data channel section in the description.");
+ error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
+ DestroyDataChannelTransport(error);
+ } else if (data_info->rejected) {
+ rtc::StringBuilder sb;
+ sb << "Rejected data channel with mid=" << data_info->name << ".";
+
+ RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release());
+ error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
+ DestroyDataChannelTransport(error);
}
}
@@ -4665,12 +4681,12 @@
}
}
-void SdpOfferAnswerHandler::DestroyDataChannelTransport() {
+void SdpOfferAnswerHandler::DestroyDataChannelTransport(RTCError error) {
RTC_DCHECK_RUN_ON(signaling_thread());
const bool has_sctp = pc_->sctp_mid().has_value();
if (has_sctp)
- data_channel_controller()->OnTransportChannelClosed();
+ data_channel_controller()->OnTransportChannelClosed(error);
pc_->network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(pc_->network_thread());
@@ -4743,7 +4759,7 @@
}
}
- DestroyDataChannelTransport();
+ DestroyDataChannelTransport({});
}
void SdpOfferAnswerHandler::GenerateMediaDescriptionOptions(
diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h
index e5b39b8..f86b900b 100644
--- a/pc/sdp_offer_answer.h
+++ b/pc/sdp_offer_answer.h
@@ -13,6 +13,7 @@
#include <stddef.h>
#include <stdint.h>
+
#include <functional>
#include <map>
#include <memory>
@@ -520,7 +521,7 @@
// Destroys the RTP data channel transport and/or the SCTP data channel
// transport and clears it.
- void DestroyDataChannelTransport();
+ void DestroyDataChannelTransport(RTCError error);
// Destroys the given ChannelInterface.
// The channel cannot be accessed after this method is called.
