Return an error when datachannel closes due to network error

This is the start of generating compliant errors, including diagnostics,
when datachannels close because of errors.

Bug: chromium:1030631
Change-Id: I39aa41728efb25bca6193a782db4cbdaad8e0dc1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161304
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30034}
This commit is contained in:
Harald Alvestrand 2019-12-08 05:55:43 +01:00 committed by Commit Bot
parent 33f9d2b383
commit dfbfb46062
8 changed files with 130 additions and 25 deletions

View file

@ -229,6 +229,7 @@ rtc_library("rtc_error") {
"../rtc_base:logging",
"../rtc_base:macromagic",
"../rtc_base/system:rtc_export",
"//third_party/abseil-cpp/absl/types:optional",
]
}

View file

@ -20,6 +20,7 @@
#include <string>
#include "absl/types/optional.h"
#include "api/rtc_error.h"
#include "rtc_base/checks.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/ref_count.h"
@ -154,6 +155,10 @@ class RTC_EXPORT DataChannelInterface : public rtc::RefCountInterface {
// determined, and until then this will return -1.
virtual int id() const = 0;
virtual DataState state() const = 0;
// When state is kClosed, and the DataChannel was not closed using
// the closing procedure, returns the error information about the closing.
// The default implementation returns "no error".
virtual RTCError error() const { return RTCError(); }
virtual uint32_t messages_sent() const = 0;
virtual uint64_t bytes_sent() const = 0;
virtual uint32_t messages_received() const = 0;

View file

@ -26,19 +26,34 @@ const char* kRTCErrorTypeNames[] = {
"NETWORK_ERROR",
"RESOURCE_EXHAUSTED",
"INTERNAL_ERROR",
"OPERATION_ERROR_WITH_DATA",
};
static_assert(static_cast<int>(webrtc::RTCErrorType::INTERNAL_ERROR) ==
(arraysize(kRTCErrorTypeNames) - 1),
"kRTCErrorTypeNames must have as many strings as RTCErrorType "
"has values.");
static_assert(
static_cast<int>(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA) ==
(arraysize(kRTCErrorTypeNames) - 1),
"kRTCErrorTypeNames must have as many strings as RTCErrorType "
"has values.");
const char* kRTCErrorDetailTypeNames[] = {
"NONE",
"DATA_CHANNEL_FAILURE",
"DTLS_FAILURE",
"FINGERPRINT_FAILURE",
"SCTP_FAILURE",
"SDP_SYNTAX_ERROR",
"HARDWARE_ENCODER_NOT_AVAILABLE",
"HARDWARE_ENCODER_ERROR",
};
static_assert(
static_cast<int>(webrtc::RTCErrorDetailType::HARDWARE_ENCODER_ERROR) ==
(arraysize(kRTCErrorDetailTypeNames) - 1),
"kRTCErrorDetailTypeNames must have as many strings as "
"RTCErrorDetailType has values.");
} // namespace
namespace webrtc {
RTCError::RTCError(RTCError&& other) = default;
RTCError& RTCError::operator=(RTCError&& other) = default;
// static
RTCError RTCError::OK() {
return RTCError();
@ -57,4 +72,9 @@ const char* ToString(RTCErrorType error) {
return kRTCErrorTypeNames[index];
}
const char* ToString(RTCErrorDetailType error) {
int index = static_cast<int>(error);
return kRTCErrorDetailTypeNames[index];
}
} // namespace webrtc

View file

@ -17,6 +17,7 @@
#include <string>
#include <utility> // For std::move.
#include "absl/types/optional.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/system/rtc_export.h"
@ -73,6 +74,25 @@ enum class RTCErrorType {
// The operation failed due to an internal error.
// Maps to OperationError DOMException.
INTERNAL_ERROR,
// An error occured that has additional data.
// The additional data is specified in
// https://w3c.github.io/webrtc-pc/#rtcerror-interface
// Maps to RTCError DOMException.
OPERATION_ERROR_WITH_DATA,
};
// Detail information, showing what further information should be present.
// https://w3c.github.io/webrtc-pc/#rtcerrordetailtype-enum
enum class RTCErrorDetailType {
NONE,
DATA_CHANNEL_FAILURE,
DTLS_FAILURE,
FINGERPRINT_FAILURE,
SCTP_FAILURE,
SDP_SYNTAX_ERROR,
HARDWARE_ENCODER_NOT_AVAILABLE,
HARDWARE_ENCODER_ERROR,
};
// Roughly corresponds to RTCError in the web api. Holds an error type, a
@ -91,15 +111,11 @@ class RTC_EXPORT RTCError {
RTCError(RTCErrorType type, std::string message)
: type_(type), message_(std::move(message)) {}
// Delete the copy constructor and assignment operator; there aren't any use
// cases where you should need to copy an RTCError, as opposed to moving it.
// Can revisit this decision if use cases arise in the future.
RTCError(const RTCError& other) = delete;
RTCError& operator=(const RTCError& other) = delete;
// Move constructor and move-assignment operator.
RTCError(RTCError&& other);
RTCError& operator=(RTCError&& other);
// In many use cases, it is better to use move than copy,
// but copy and assignment are provided for those cases that need it.
// Note that this has extra overhead because it copies strings.
RTCError(const RTCError& other) = default;
RTCError& operator=(const RTCError& other) = default;
// Identical to default constructed error.
//
@ -117,6 +133,13 @@ class RTC_EXPORT RTCError {
void set_message(std::string message);
RTCErrorDetailType error_detail() const { return error_detail_; }
void set_error_detail(RTCErrorDetailType detail) { error_detail_ = detail; }
absl::optional<uint16_t> sctp_cause_code() { return sctp_cause_code_; }
void set_sctp_cause_code(uint16_t cause_code) {
sctp_cause_code_ = cause_code;
}
// Convenience method for situations where you only care whether or not an
// error occurred.
bool ok() const { return type_ == RTCErrorType::NONE; }
@ -124,6 +147,8 @@ class RTC_EXPORT RTCError {
private:
RTCErrorType type_ = RTCErrorType::NONE;
std::string message_;
RTCErrorDetailType error_detail_ = RTCErrorDetailType::NONE;
absl::optional<uint16_t> sctp_cause_code_;
};
// Outputs the error as a friendly string. Update this method when adding a new
@ -132,6 +157,7 @@ class RTC_EXPORT RTCError {
// Only intended to be used for logging/diagnostics. The returned char* points
// to literal string that lives for the whole duration of the program.
RTC_EXPORT const char* ToString(RTCErrorType error);
RTC_EXPORT const char* ToString(RTCErrorDetailType error);
#ifdef UNIT_TEST
inline std::ostream& operator<<( // no-presubmit-check TODO(webrtc:8982)
@ -139,6 +165,12 @@ inline std::ostream& operator<<( // no-presubmit-check TODO(webrtc:8982)
RTCErrorType error) {
return stream << ToString(error);
}
inline std::ostream& operator<<( // no-presubmit-check TODO(webrtc:8982)
std::ostream& stream, // no-presubmit-check TODO(webrtc:8982)
RTCErrorDetailType error) {
return stream << ToString(error);
}
#endif // UNIT_TEST
// Helper macro that can be used by implementations to create an error with a

View file

@ -261,6 +261,10 @@ void DataChannel::Close() {
UpdateState();
}
RTCError DataChannel::error() const {
return error_;
}
bool DataChannel::Send(const DataBuffer& buffer) {
buffered_amount_ += buffer.size();
if (state_ != kOpen) {
@ -283,7 +287,10 @@ bool DataChannel::Send(const DataBuffer& buffer) {
if (!QueueSendDataMessage(buffer)) {
RTC_LOG(LS_ERROR) << "Closing the DataChannel due to a failure to queue "
"additional data.";
CloseAbruptly();
// https://w3c.github.io/webrtc-pc/#dom-rtcdatachannel-send step 5
// Note that the spec doesn't explicitly say to close in this situation.
CloseAbruptlyWithError(RTCError(RTCErrorType::RESOURCE_EXHAUSTED,
"Unable to queue data for sending"));
}
return true;
}
@ -363,13 +370,17 @@ void DataChannel::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.
CloseAbruptly();
// Note: this needs to differentiate between normal close and error close.
// https://w3c.github.io/webrtc-pc/#announcing-a-data-channel-as-closed
CloseAbruptlyWithError(
RTCError(RTCErrorType::NETWORK_ERROR, "Transport channel closed"));
}
// The remote peer request that this channel shall be closed.
void DataChannel::RemotePeerRequestClose() {
RTC_DCHECK(data_channel_type_ == cricket::DCT_RTP);
CloseAbruptly();
// Close with error code explicitly set to OK.
CloseAbruptlyWithError(RTCError());
}
void DataChannel::SetSendSsrc(uint32_t send_ssrc) {
@ -438,7 +449,9 @@ void DataChannel::OnDataReceived(const cricket::ReceiveDataParams& params,
queued_received_data_.Clear();
if (data_channel_type_ != cricket::DCT_RTP) {
CloseAbruptly();
CloseAbruptlyWithError(
RTCError(RTCErrorType::RESOURCE_EXHAUSTED,
"Queued received data exceeds the max buffer size."));
}
return;
@ -458,7 +471,7 @@ void DataChannel::OnChannelReady(bool writable) {
UpdateState();
}
void DataChannel::CloseAbruptly() {
void DataChannel::CloseAbruptlyWithError(RTCError error) {
if (state_ == kClosed) {
return;
}
@ -475,9 +488,17 @@ void DataChannel::CloseAbruptly() {
// Still go to "kClosing" before "kClosed", since observers may be expecting
// that.
SetState(kClosing);
error_ = std::move(error);
SetState(kClosed);
}
void DataChannel::CloseAbruptlyWithDataChannelFailure(
const std::string& message) {
RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, message);
error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
CloseAbruptlyWithError(std::move(error));
}
void DataChannel::UpdateState() {
// UpdateState determines what to do from a few state variables. Include
// all conditions required for each state transition here for
@ -652,7 +673,8 @@ bool DataChannel::SendDataMessage(const DataBuffer& buffer,
RTC_LOG(LS_ERROR) << "Closing the DataChannel due to a failure to send data, "
"send_result = "
<< send_result;
CloseAbruptly();
CloseAbruptlyWithError(
RTCError(RTCErrorType::NETWORK_ERROR, "Failure to send data"));
return false;
}
@ -713,7 +735,8 @@ bool DataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
RTC_LOG(LS_ERROR) << "Closing the DataChannel due to a failure to send"
" the CONTROL message, send_result = "
<< send_result;
CloseAbruptly();
CloseAbruptlyWithError(RTCError(RTCErrorType::NETWORK_ERROR,
"Failed to send a CONTROL message"));
}
return retval;
}

View file

@ -146,6 +146,7 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
virtual uint64_t buffered_amount() const;
virtual void Close();
virtual DataState state() const { return state_; }
virtual RTCError error() const;
virtual uint32_t messages_sent() const { return messages_sent_; }
virtual uint64_t bytes_sent() const { return bytes_sent_; }
virtual uint32_t messages_received() const { return messages_received_; }
@ -157,7 +158,11 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
// be removed, or SCTP data channels when the underlying SctpTransport is
// being destroyed.
// It is also called by the PeerConnection if SCTP ID assignment fails.
void CloseAbruptly();
void CloseAbruptlyWithError(RTCError error);
// Specializations of CloseAbruptlyWithError
void CloseAbruptlyWithDataChannelFailure(const std::string& message);
void CloseAbruptlyWithSctpCauseCode(const std::string& message,
uint16_t cause_code);
// Called when the channel's ready to use. That can happen when the
// underlying DataMediaChannel becomes ready, or when this channel is a new
@ -277,6 +282,7 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
InternalDataChannelInit config_;
DataChannelObserver* observer_;
DataState state_;
RTCError error_;
uint32_t messages_sent_;
uint64_t bytes_sent_;
uint32_t messages_received_;
@ -319,6 +325,7 @@ PROXY_CONSTMETHOD0(std::string, protocol)
PROXY_CONSTMETHOD0(bool, negotiated)
PROXY_CONSTMETHOD0(int, id)
PROXY_CONSTMETHOD0(DataState, state)
PROXY_CONSTMETHOD0(RTCError, error)
PROXY_CONSTMETHOD0(uint32_t, messages_sent)
PROXY_CONSTMETHOD0(uint64_t, bytes_sent)
PROXY_CONSTMETHOD0(uint32_t, messages_received)

View file

@ -330,7 +330,7 @@ void DataChannelController::AllocateSctpSids(rtc::SSLRole role) {
// Since closing modifies the list of channels, we have to do the actual
// closing outside the loop.
for (const auto& channel : channels_to_close) {
channel->CloseAbruptly();
channel->CloseAbruptlyWithDataChannelFailure("Failed to allocate SCTP SID");
}
}

View file

@ -152,6 +152,7 @@ TEST_F(SctpDataChannelTest, StateTransition) {
webrtc_data_channel_->Close();
EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state());
EXPECT_TRUE(webrtc_data_channel_->error().ok());
EXPECT_EQ(state_signals_listener.opened_count(), 1);
EXPECT_EQ(state_signals_listener.closed_count(), 1);
// Verifies that it's disconnected from the transport.
@ -395,6 +396,7 @@ TEST_F(SctpDataChannelTest, QueuedCloseFlushes) {
provider_->set_send_blocked(false);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state(), 1000);
EXPECT_TRUE(webrtc_data_channel_->error().ok());
EXPECT_EQ(cricket::DMT_TEXT, provider_->last_send_data_params().type);
}
@ -560,6 +562,11 @@ TEST_F(SctpDataChannelTest, ClosedOnTransportError) {
EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state());
EXPECT_FALSE(webrtc_data_channel_->error().ok());
EXPECT_EQ(webrtc::RTCErrorType::NETWORK_ERROR,
webrtc_data_channel_->error().type());
EXPECT_EQ(webrtc::RTCErrorDetailType::NONE,
webrtc_data_channel_->error().error_detail());
}
// Tests that the DataChannel is closed if the received buffer is full.
@ -577,6 +584,11 @@ TEST_F(SctpDataChannelTest, ClosedWhenReceivedBufferFull) {
}
EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state());
EXPECT_FALSE(webrtc_data_channel_->error().ok());
EXPECT_EQ(webrtc::RTCErrorType::RESOURCE_EXHAUSTED,
webrtc_data_channel_->error().type());
EXPECT_EQ(webrtc::RTCErrorDetailType::NONE,
webrtc_data_channel_->error().error_detail());
}
// Tests that sending empty data returns no error and keeps the channel open.
@ -617,6 +629,11 @@ TEST_F(SctpDataChannelTest, TransportDestroyedWhileDataBuffered) {
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::NETWORK_ERROR,
webrtc_data_channel_->error().type());
EXPECT_EQ(webrtc::RTCErrorDetailType::NONE,
webrtc_data_channel_->error().error_detail());
}
class SctpSidAllocatorTest : public ::testing::Test {