Remove Try from Write methods (#614)

- We write to a Stream so it should never fail (it could throw
but that's not new).
This commit is contained in:
David Fowler 2017-06-28 21:53:23 -07:00 committed by GitHub
parent d6f5e16f38
commit 6332e98d03
21 changed files with 43 additions and 75 deletions

View File

@ -81,7 +81,7 @@ namespace Microsoft.AspNetCore.SignalR.Client
using (var memoryStream = new MemoryStream()) using (var memoryStream = new MemoryStream())
{ {
NegotiationProtocol.TryWriteProtocolNegotiationMessage(new NegotiationMessage(_protocol.Name), memoryStream); NegotiationProtocol.WriteMessage(new NegotiationMessage(_protocol.Name), memoryStream);
await _connection.SendAsync(memoryStream.ToArray(), _connectionActive.Token); await _connection.SendAsync(memoryStream.ToArray(), _connectionActive.Token);
} }
} }

View File

@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
{ {
public static class BinaryMessageFormatter public static class BinaryMessageFormatter
{ {
public static bool TryWriteMessage(ReadOnlySpan<byte> payload, Stream output) public static void WriteMessage(ReadOnlySpan<byte> payload, Stream output)
{ {
// TODO: Optimize for size - (e.g. use Varints) // TODO: Optimize for size - (e.g. use Varints)
var length = sizeof(long); var length = sizeof(long);
@ -23,8 +23,6 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
payload.CopyTo(buffer); payload.CopyTo(buffer);
output.Write(buffer, 0, payload.Length); output.Write(buffer, 0, payload.Length);
ArrayPool<byte>.Shared.Return(buffer); ArrayPool<byte>.Shared.Return(buffer);
return true;
} }
} }
} }

View File

@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
internal const char FieldDelimiter = ':'; internal const char FieldDelimiter = ':';
internal const char MessageDelimiter = ';'; internal const char MessageDelimiter = ';';
public static bool TryWriteMessage(ReadOnlySpan<byte> payload, Stream output) public static void WriteMessage(ReadOnlySpan<byte> payload, Stream output)
{ {
// Calculate the length, it's the number of characters for text messages, but number of base64 characters for binary // Calculate the length, it's the number of characters for text messages, but number of base64 characters for binary
@ -39,8 +39,6 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
// Terminator // Terminator
output.WriteByte((byte)MessageDelimiter); output.WriteByte((byte)MessageDelimiter);
return true;
} }
} }
} }

View File

@ -13,10 +13,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
using (var output = new MemoryStream()) using (var output = new MemoryStream())
{ {
// Encode the message // Encode the message
if (!protocol.TryWriteMessage(message, output)) protocol.WriteMessage(message, output);
{
throw new InvalidOperationException("Failed to write message to the output stream");
}
return output.ToArray(); return output.ToArray();
} }

View File

@ -13,6 +13,6 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
bool TryParseMessages(ReadOnlySpan<byte> input, IInvocationBinder binder, out IList<HubMessage> messages); bool TryParseMessages(ReadOnlySpan<byte> input, IInvocationBinder binder, out IList<HubMessage> messages);
bool TryWriteMessage(HubMessage message, Stream output); void WriteMessage(HubMessage message, Stream output);
} }
} }

View File

@ -63,14 +63,14 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
return messages.Count > 0; return messages.Count > 0;
} }
public bool TryWriteMessage(HubMessage message, Stream output) public void WriteMessage(HubMessage message, Stream output)
{ {
using (var memoryStream = new MemoryStream()) using (var memoryStream = new MemoryStream())
{ {
WriteMessage(message, memoryStream); WriteMessageCore(message, memoryStream);
memoryStream.Flush(); memoryStream.Flush();
return TextMessageFormatter.TryWriteMessage(memoryStream.ToArray(), output); TextMessageFormatter.WriteMessage(memoryStream.ToArray(), output);
} }
} }
@ -111,7 +111,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
} }
} }
private void WriteMessage(HubMessage message, Stream stream) private void WriteMessageCore(HubMessage message, Stream stream)
{ {
using (var writer = new JsonTextWriter(new StreamWriter(stream))) using (var writer = new JsonTextWriter(new StreamWriter(stream)))
{ {

View File

@ -107,16 +107,16 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
return new CompletionMessage(invocationId, error, result, hasResult); return new CompletionMessage(invocationId, error, result, hasResult);
} }
public bool TryWriteMessage(HubMessage message, Stream output) public void WriteMessage(HubMessage message, Stream output)
{ {
using (var memoryStream = new MemoryStream()) using (var memoryStream = new MemoryStream())
{ {
WriteMessage(message, memoryStream); WriteMessageCore(message, memoryStream);
return BinaryMessageFormatter.TryWriteMessage(new ReadOnlySpan<byte>(memoryStream.ToArray()), output); BinaryMessageFormatter.WriteMessage(new ReadOnlySpan<byte>(memoryStream.ToArray()), output);
} }
} }
private void WriteMessage(HubMessage message, Stream output) private void WriteMessageCore(HubMessage message, Stream output)
{ {
var packer = Packer.Create(output); var packer = Packer.Create(output);
switch (message) switch (message)

View File

@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
{ {
private const string ProtocolPropertyName = "protocol"; private const string ProtocolPropertyName = "protocol";
public static bool TryWriteProtocolNegotiationMessage(NegotiationMessage negotiationMessage, Stream output) public static void WriteMessage(NegotiationMessage negotiationMessage, Stream output)
{ {
using (var memoryStream = new MemoryStream()) using (var memoryStream = new MemoryStream())
{ {
@ -25,12 +25,11 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
writer.WriteEndObject(); writer.WriteEndObject();
} }
memoryStream.Flush(); TextMessageFormatter.WriteMessage(memoryStream.ToArray(), output);
return TextMessageFormatter.TryWriteMessage(new ReadOnlySpan<byte>(memoryStream.ToArray()), output);
} }
} }
public static bool TryReadProtocolNegotiationMessage(ReadOnlySpan<byte> input, out NegotiationMessage negotiationMessage) public static bool TryParseMessage(ReadOnlySpan<byte> input, out NegotiationMessage negotiationMessage)
{ {
var parser = new TextMessageParser(); var parser = new TextMessageParser();
if (!parser.TryParseMessage(ref input, out var payload)) if (!parser.TryParseMessage(ref input, out var payload))

View File

@ -78,7 +78,7 @@ namespace Microsoft.AspNetCore.SignalR
{ {
while (connection.Transport.In.TryRead(out var buffer)) while (connection.Transport.In.TryRead(out var buffer))
{ {
if (NegotiationProtocol.TryReadProtocolNegotiationMessage(buffer, out var negotiationMessage)) if (NegotiationProtocol.TryParseMessage(buffer, out var negotiationMessage))
{ {
// Resolve the Hub Protocol for the connection and store it in metadata // Resolve the Hub Protocol for the connection and store it in metadata
// Other components, outside the Hub, may need to know what protocol is in use // Other components, outside the Hub, may need to know what protocol is in use

View File

@ -14,25 +14,21 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
private const byte LineFeed = (byte)'\n'; private const byte LineFeed = (byte)'\n';
public static bool TryWriteMessage(ReadOnlySpan<byte> payload, MemoryStream output) public static void WriteMessage(ReadOnlySpan<byte> payload, MemoryStream output)
{ {
// Write the payload // Write the payload
if (!TryWritePayload(payload, output)) WritePayload(payload, output);
{
return false;
}
// Write new \r\n
output.Write(Newline, 0, Newline.Length); output.Write(Newline, 0, Newline.Length);
return true;
} }
private static bool TryWritePayload(ReadOnlySpan<byte> payload, Stream output) private static void WritePayload(ReadOnlySpan<byte> payload, Stream output)
{ {
// Short-cut for empty payload // Short-cut for empty payload
if (payload.Length == 0) if (payload.Length == 0)
{ {
return true; return;
} }
// We can't just use while(payload.Length > 0) because we need to write a blank final "data: " line // We can't just use while(payload.Length > 0) because we need to write a blank final "data: " line
@ -76,16 +72,11 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
payload = payload.Slice(nextSliceStart); payload = payload.Slice(nextSliceStart);
} }
if (!TryWriteLine(slice, output)) WriteLine(slice, output);
{
return false;
}
} }
return true;
} }
private static bool TryWriteLine(ReadOnlySpan<byte> payload, Stream output) private static void WriteLine(ReadOnlySpan<byte> payload, Stream output)
{ {
output.Write(DataPrefix, 0, DataPrefix.Length); output.Write(DataPrefix, 0, DataPrefix.Length);
@ -95,8 +86,6 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Formatters
ArrayPool<byte>.Shared.Return(buffer); ArrayPool<byte>.Shared.Return(buffer);
output.Write(Newline, 0, Newline.Length); output.Write(Newline, 0, Newline.Length);
return true;
} }
} }
} }

View File

@ -50,16 +50,8 @@ namespace Microsoft.AspNetCore.Sockets.Internal.Transports
while (_application.TryRead(out var buffer)) while (_application.TryRead(out var buffer))
{ {
_logger.SSEWritingMessage(_connectionId, buffer.Length); _logger.SSEWritingMessage(_connectionId, buffer.Length);
if (!ServerSentEventsMessageFormatter.TryWriteMessage(buffer, ms))
{
// We ran out of space to write, even after trying to enlarge.
// This should only happen in a significant lack-of-memory scenario.
// IOutput doesn't really have a way to write incremental ServerSentEventsMessageFormatter.WriteMessage(buffer, ms);
// Throwing InvalidOperationException here, but it's not quite an invalid operation...
throw new InvalidOperationException("Ran out of space to format messages!");
}
} }
ms.Seek(0, SeekOrigin.Begin); ms.Seek(0, SeekOrigin.Begin);

View File

@ -193,7 +193,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
throw new InvalidOperationException("No Parsed Message provided"); throw new InvalidOperationException("No Parsed Message provided");
} }
public bool TryWriteMessage(HubMessage message, Stream output) public void WriteMessage(HubMessage message, Stream output)
{ {
WriteCalls += 1; WriteCalls += 1;
@ -201,7 +201,6 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
{ {
throw _error; throw _error;
} }
return true;
} }
} }
} }

View File

@ -87,7 +87,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
private byte[] FormatMessageToArray(byte[] message) private byte[] FormatMessageToArray(byte[] message)
{ {
var output = new MemoryStream(); var output = new MemoryStream();
Assert.True(TextMessageFormatter.TryWriteMessage(message, output)); TextMessageFormatter.WriteMessage(message, output);
return output.ToArray(); return output.ToArray();
} }

View File

@ -143,7 +143,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
private static byte[] FormatMessageToArray(byte[] message) private static byte[] FormatMessageToArray(byte[] message)
{ {
var output = new MemoryStream(); var output = new MemoryStream();
Assert.True(TextMessageFormatter.TryWriteMessage(message, output)); TextMessageFormatter.WriteMessage(message, output);
return output.ToArray(); return output.ToArray();
} }
} }

View File

@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
{ {
foreach (var hubMessage in hubMessages) foreach (var hubMessage in hubMessages)
{ {
_hubProtocol.TryWriteMessage(hubMessage, memoryStream); _hubProtocol.WriteMessage(hubMessage, memoryStream);
} }
_hubProtocol.TryParseMessages( _hubProtocol.TryParseMessages(

View File

@ -17,8 +17,8 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
var negotiationMessage = new NegotiationMessage(protocol: "dummy"); var negotiationMessage = new NegotiationMessage(protocol: "dummy");
using (var ms = new MemoryStream()) using (var ms = new MemoryStream())
{ {
Assert.True(NegotiationProtocol.TryWriteProtocolNegotiationMessage(negotiationMessage, ms)); NegotiationProtocol.WriteMessage(negotiationMessage, ms);
Assert.True(NegotiationProtocol.TryReadProtocolNegotiationMessage(ms.ToArray(), out var deserializedMessage)); Assert.True(NegotiationProtocol.TryParseMessage(ms.ToArray(), out var deserializedMessage));
Assert.NotNull(deserializedMessage); Assert.NotNull(deserializedMessage);
Assert.Equal(negotiationMessage.Protocol, deserializedMessage.Protocol); Assert.Equal(negotiationMessage.Protocol, deserializedMessage.Protocol);
@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
var message = Encoding.UTF8.GetBytes(payload); var message = Encoding.UTF8.GetBytes(payload);
var exception = Assert.Throws<FormatException>(() => var exception = Assert.Throws<FormatException>(() =>
Assert.True(NegotiationProtocol.TryReadProtocolNegotiationMessage(message, out var deserializedMessage))); Assert.True(NegotiationProtocol.TryParseMessage(message, out var deserializedMessage)));
Assert.Equal(expectedMessage, exception.Message); Assert.Equal(expectedMessage, exception.Message);
} }

View File

@ -27,20 +27,14 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks
var buffer = new byte[MessageLength]; var buffer = new byte[MessageLength];
Random.NextBytes(buffer); Random.NextBytes(buffer);
var output = new MemoryStream(); var output = new MemoryStream();
if (!BinaryMessageFormatter.TryWriteMessage(buffer, output)) BinaryMessageFormatter.WriteMessage(buffer, output);
{
throw new InvalidOperationException("Failed to format message");
}
_binaryInput = output.ToArray(); _binaryInput = output.ToArray();
buffer = new byte[MessageLength]; buffer = new byte[MessageLength];
Random.NextBytes(buffer); Random.NextBytes(buffer);
output = new MemoryStream(); output = new MemoryStream();
if (!TextMessageFormatter.TryWriteMessage(buffer, output)) TextMessageFormatter.WriteMessage(buffer, output);
{
throw new InvalidOperationException("Failed to format message");
}
_textInput = output.ToArray(); _textInput = output.ToArray();
} }

View File

@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters
var output = new MemoryStream(); // Use small chunks to test Advance/Enlarge and partial payload writing var output = new MemoryStream(); // Use small chunks to test Advance/Enlarge and partial payload writing
foreach (var message in messages) foreach (var message in messages)
{ {
Assert.True(BinaryMessageFormatter.TryWriteMessage(message, output)); BinaryMessageFormatter.WriteMessage(message, output);
} }
Assert.Equal(expectedEncoding, output.ToArray()); Assert.Equal(expectedEncoding, output.ToArray());
@ -54,7 +54,8 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters
output.Seek(offset, SeekOrigin.Begin); output.Seek(offset, SeekOrigin.Begin);
} }
Assert.True(BinaryMessageFormatter.TryWriteMessage(payload, output)); BinaryMessageFormatter.WriteMessage(payload, output);
Assert.Equal(encoded, output.ToArray().Slice(offset).ToArray()); Assert.Equal(encoded, output.ToArray().Slice(offset).ToArray());
} }
@ -74,7 +75,8 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters
output.Seek(offset, SeekOrigin.Begin); output.Seek(offset, SeekOrigin.Begin);
} }
Assert.True(BinaryMessageFormatter.TryWriteMessage(message, output)); BinaryMessageFormatter.WriteMessage(message, output);
Assert.Equal(encoded, output.ToArray().Slice(offset).ToArray()); Assert.Equal(encoded, output.ToArray().Slice(offset).ToArray());
} }
} }

View File

@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters
var output = new MemoryStream(); var output = new MemoryStream();
foreach (var message in messages) foreach (var message in messages)
{ {
Assert.True(TextMessageFormatter.TryWriteMessage(message, output)); TextMessageFormatter.WriteMessage(message, output);
} }
Assert.Equal(expectedEncoding, Encoding.UTF8.GetString(output.ToArray())); Assert.Equal(expectedEncoding, Encoding.UTF8.GetString(output.ToArray()));
@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters
var message = Encoding.UTF8.GetBytes(payload); var message = Encoding.UTF8.GetBytes(payload);
var output = new MemoryStream(); var output = new MemoryStream();
Assert.True(TextMessageFormatter.TryWriteMessage(message, output)); TextMessageFormatter.WriteMessage(message, output);
Assert.Equal(encoded, Encoding.UTF8.GetString(output.ToArray())); Assert.Equal(encoded, Encoding.UTF8.GetString(output.ToArray()));
} }

View File

@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests
using (var memoryStream = new MemoryStream()) using (var memoryStream = new MemoryStream())
{ {
NegotiationProtocol.TryWriteProtocolNegotiationMessage(new NegotiationMessage(_protocol.Name), memoryStream); NegotiationProtocol.WriteMessage(new NegotiationMessage(_protocol.Name), memoryStream);
Application.Out.TryWrite(memoryStream.ToArray()); Application.Out.TryWrite(memoryStream.ToArray());
} }
} }

View File

@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters
public void WriteTextMessage(string encoded, string payload) public void WriteTextMessage(string encoded, string payload)
{ {
var output = new MemoryStream(); var output = new MemoryStream();
Assert.True(ServerSentEventsMessageFormatter.TryWriteMessage(Encoding.UTF8.GetBytes(payload), output)); ServerSentEventsMessageFormatter.WriteMessage(Encoding.UTF8.GetBytes(payload), output);
Assert.Equal(encoded, Encoding.UTF8.GetString(output.ToArray())); Assert.Equal(encoded, Encoding.UTF8.GetString(output.ToArray()));
} }