Skip to content

Simplify catch-rethrow logic in NetworkStream #44246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 6, 2020
Merged

Simplify catch-rethrow logic in NetworkStream #44246

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7d2d6aa
fix: NetworkStream throwing inconsistent exceptions
antonfirsov Aug 13, 2020
3c847a5
no more nested NetworkExceptions
antonfirsov Aug 13, 2020
875be8d
Merge branch 'exp/SendToRecvFrom_Datagram_UDP' into af/fix-NetworkStr…
antonfirsov Nov 3, 2020
b90a9a8
fix exception message and update code
antonfirsov Nov 3, 2020
94e243c
fix test
antonfirsov Nov 3, 2020
d0f2e7a
refactor WIP
antonfirsov Nov 4, 2020
31c20a4
Merge branch 'master' into tests/NetworkStream-exceptions-refactor
antonfirsov Nov 4, 2020
63e23b5
simplify catch-rethrow logic in NetworkStream
antonfirsov Nov 4, 2020
1f59192
improve test
antonfirsov Nov 4, 2020
2160fe7
Merge branch 'master' into tests/NetworkStream-exceptions-refactor
antonfirsov Nov 4, 2020
9f00802
Merge branch 'master' into tests/NetworkStream-exceptions-refactor
antonfirsov Nov 5, 2020
51a4668
do not Assert on exception.Message content
antonfirsov Nov 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 19 additions & 80 deletions src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ public NetworkStream(Socket socket, FileAccess access, bool ownsSocket)
// allowing non-blocking sockets could result in non-deterministic failures from those
// operations. A developer that requires using NetworkStream with a non-blocking socket can
// temporarily flip Socket.Blocking as a workaround.
throw GetCustomException(SR.net_sockets_blocking);
throw new IOException(SR.net_sockets_blocking);
}
if (!socket.Connected)
{
throw GetCustomException(SR.net_notconnected);
throw new IOException(SR.net_notconnected);
}
if (socket.SocketType != SocketType.Stream)
{
throw GetCustomException(SR.net_notstream);
throw new IOException(SR.net_notstream);
}

_streamSocket = socket;
Expand Down Expand Up @@ -227,13 +227,9 @@ public override int Read(byte[] buffer, int offset, int count)
{
return _streamSocket.Receive(buffer, offset, count, 0);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_readfailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception);
throw WrapException(SR.net_io_readfailure, exception);
}
}

Expand All @@ -250,23 +246,14 @@ public override int Read(Span<byte> buffer)
ThrowIfDisposed();
if (!CanRead) throw new InvalidOperationException(SR.net_writeonlystream);

int bytesRead;
SocketError errorCode;
try
{
bytesRead = _streamSocket.Receive(buffer, SocketFlags.None, out errorCode);
return _streamSocket.Receive(buffer, SocketFlags.None);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception);
}

if (errorCode != SocketError.Success)
{
var socketException = new SocketException((int)errorCode);
throw GetExceptionFromSocketException(SR.Format(SR.net_io_readfailure, socketException.Message), socketException);
throw WrapException(SR.net_io_readfailure, exception);
}
return bytesRead;
}

public override unsafe int ReadByte()
Expand Down Expand Up @@ -306,13 +293,9 @@ public override void Write(byte[] buffer, int offset, int count)
// after ALL the requested number of bytes was transferred.
_streamSocket.Send(buffer, offset, count, SocketFlags.None);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_writefailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception);
throw WrapException(SR.net_io_writefailure, exception);
}
}

Expand All @@ -330,20 +313,13 @@ public override void Write(ReadOnlySpan<byte> buffer)
ThrowIfDisposed();
if (!CanWrite) throw new InvalidOperationException(SR.net_readonlystream);

SocketError errorCode;
try
{
_streamSocket.Send(buffer, SocketFlags.None, out errorCode);
_streamSocket.Send(buffer, SocketFlags.None);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception);
}

if (errorCode != SocketError.Success)
{
var socketException = new SocketException((int)errorCode);
throw GetExceptionFromSocketException(SR.Format(SR.net_io_writefailure, socketException.Message), socketException);
throw WrapException(SR.net_io_writefailure, exception);
}
}

Expand Down Expand Up @@ -424,13 +400,9 @@ public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, Asy
callback,
state);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_readfailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception);
throw WrapException(SR.net_io_readfailure, exception);
}
}

Expand All @@ -456,13 +428,9 @@ public override int EndRead(IAsyncResult asyncResult)
{
return _streamSocket.EndReceive(asyncResult);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_readfailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception);
throw WrapException(SR.net_io_readfailure, exception);
}
}

Expand Down Expand Up @@ -500,13 +468,9 @@ public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, As
callback,
state);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_writefailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception);
throw WrapException(SR.net_io_writefailure, exception);
}
}

Expand All @@ -528,13 +492,9 @@ public override void EndWrite(IAsyncResult asyncResult)
{
_streamSocket.EndSend(asyncResult);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_writefailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception);
throw WrapException(SR.net_io_writefailure, exception);
}
}

Expand Down Expand Up @@ -570,13 +530,9 @@ public override Task<int> ReadAsync(byte[] buffer, int offset, int count, Cancel
fromNetworkStream: true,
cancellationToken).AsTask();
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_readfailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception);
throw WrapException(SR.net_io_readfailure, exception);
}
}

Expand All @@ -597,13 +553,9 @@ public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken
fromNetworkStream: true,
cancellationToken: cancellationToken);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_readfailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception);
throw WrapException(SR.net_io_readfailure, exception);
}
}

Expand Down Expand Up @@ -638,13 +590,9 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati
SocketFlags.None,
cancellationToken).AsTask();
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_writefailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception);
throw WrapException(SR.net_io_writefailure, exception);
}
}

Expand All @@ -664,13 +612,9 @@ public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationTo
SocketFlags.None,
cancellationToken);
}
catch (SocketException socketException)
{
throw GetExceptionFromSocketException(SR.Format(SR.net_io_writefailure, socketException.Message), socketException);
}
catch (Exception exception) when (!(exception is OutOfMemoryException))
{
throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception);
throw WrapException(SR.net_io_writefailure, exception);
}
}

Expand Down Expand Up @@ -728,14 +672,9 @@ private void ThrowIfDisposed()
void ThrowObjectDisposedException() => throw new ObjectDisposedException(GetType().FullName);
}

private static IOException GetExceptionFromSocketException(string message, SocketException innerException)
{
return new IOException(message, innerException);
}

private static IOException GetCustomException(string message, Exception? innerException = null)
private static IOException WrapException(string resourceFormatString, Exception innerException)
{
return new IOException(message, innerException);
return new IOException(SR.Format(resourceFormatString, innerException.Message), innerException);
}
}
}
28 changes: 19 additions & 9 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,21 +321,31 @@ public async Task DisposeSocketDirectly_ReadWriteThrowNetworkException(bool deri
Task<Socket> acceptTask = listener.AcceptAsync();
await Task.WhenAll(acceptTask, client.ConnectAsync(new IPEndPoint(IPAddress.Loopback, ((IPEndPoint)listener.LocalEndPoint).Port)));
using Socket serverSocket = await acceptTask;

using NetworkStream server = derivedNetworkStream ? (NetworkStream)new DerivedNetworkStream(serverSocket) : new NetworkStream(serverSocket);

serverSocket.Dispose();

Assert.Throws<IOException>(() => server.Read(new byte[1], 0, 1));
Assert.Throws<IOException>(() => server.Write(new byte[1], 0, 1));
ExpectIOException(() => server.Read(new byte[1], 0, 1), isWriteOperation: false);
ExpectIOException(() => server.Write(new byte[1], 0, 1), isWriteOperation: true);

Assert.Throws<IOException>(() => server.Read((Span<byte>)new byte[1]));
Assert.Throws<IOException>(() => server.Write((ReadOnlySpan<byte>)new byte[1]));
ExpectIOException(() => server.Read((Span<byte>)new byte[1]), isWriteOperation: false);
ExpectIOException(() => server.Write((ReadOnlySpan<byte>)new byte[1]), isWriteOperation: true);

Assert.Throws<IOException>(() => server.BeginRead(new byte[1], 0, 1, null, null));
Assert.Throws<IOException>(() => server.BeginWrite(new byte[1], 0, 1, null, null));
ExpectIOException(() => server.BeginRead(new byte[1], 0, 1, null, null), isWriteOperation: false);
ExpectIOException(() => server.BeginWrite(new byte[1], 0, 1, null, null), isWriteOperation: true);

Assert.Throws<IOException>(() => { server.ReadAsync(new byte[1], 0, 1); });
Assert.Throws<IOException>(() => { server.WriteAsync(new byte[1], 0, 1); });
ExpectIOException(() => { _ = server.ReadAsync(new byte[1], 0, 1); }, isWriteOperation: false);
ExpectIOException(() => { _ = server.WriteAsync(new byte[1], 0, 1); }, isWriteOperation: true);
}

static void ExpectIOException(Action action, bool isWriteOperation)
{
IOException ex = Assert.Throws<IOException>(action);
string expectedSubstring = isWriteOperation ? "write data" : "read data";
Assert.Contains(expectedSubstring, ex.Message);
Assert.NotNull(ex.InnerException);
Assert.IsType<ObjectDisposedException>(ex.InnerException);
}
}

Expand Down