Skip to content

SignalR additional xml docs #7980

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.

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 2 commits into from
Mar 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 31 additions & 4 deletions src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,40 @@ public partial class HubConnection
private ConnectionState _connectionState;
private int _serverProtocolMinorVersion;

/// <summary>
/// Occurs when the connection is closed. The connection could be closed due to an error or due to either the server or client intentionally
/// closing the connection without error.
/// </summary>
/// <remarks>
/// If this event was triggered from a connection error, the <see cref="Exception"/> that occurred will be passed in as the
/// sole argument to this handler. If this event was triggered intentionally by either the client or server, then
/// the argument will be <see langword="null"/>.
/// </remarks>
/// <example>
/// The following example attaches a handler to the <see cref="Closed"/> event, and checks the provided argument to determine
/// if there was an error:
///
/// <code>
/// connection.Closed += (exception) =>
/// {
/// if (exception == null)
/// {
/// Console.WriteLine("Connection closed without error.");
/// }
/// else
/// {
/// Console.WriteLine($"Connection closed due to an error: {exception.Message}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I normally prefer printing $"{exception}" over $"{exception.Message}" so you get the Exception type and stack trace too.

/// }
/// };
/// </code>
/// </example>
public event Func<Exception, Task> Closed;

// internal for testing purposes
internal TimeSpan TickRate { get; set; } = TimeSpan.FromSeconds(1);

/// <summary>
/// Gets or sets the server timeout interval for the connection.
/// Gets or sets the server timeout interval for the connection.
/// </summary>
/// <remarks>
/// The client times out if it hasn't heard from the server for `this` long.
Expand Down Expand Up @@ -510,7 +537,7 @@ private void LaunchStreams(Dictionary<string, object> readers, CancellationToken
}
}

// this is called via reflection using the `_sendStreamItems` field
// this is called via reflection using the `_sendStreamItems` field
private async Task SendStreamItems<T>(string streamId, ChannelReader<T> reader, CancellationToken token)
{
Log.StartingStream(_logger, streamId);
Expand Down Expand Up @@ -849,7 +876,7 @@ private async Task HandshakeAsync(ConnectionState startingConnectionState, Cance
}
}
}

// shutdown if we're unable to read handshake
// Ignore HubException because we throw it when we receive a handshake response with an error
// And because we already have the error, we don't need to log that the handshake failed
Expand Down Expand Up @@ -1139,7 +1166,7 @@ public void Dispose()
private class InvocationHandlerList
{
private readonly List<InvocationHandler> _invocationHandlers;
// A lazy cached copy of the handlers that doesn't change for thread safety.
// A lazy cached copy of the handlers that doesn't change for thread safety.
// Adding or removing a handler sets this to null.
private InvocationHandler[] _copiedHandlers;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ namespace Microsoft.AspNetCore.Http.Connections
{
public static class HttpConnectionContextExtensions
{
/// <summary>
/// Gets the <see cref="HttpContext"/> associated with the connection, if there is one.
/// </summary>
/// <param name="connection">The <see cref="ConnectionContext"/> representing the connection.</param>
/// <returns>The <see cref="HttpContext"/> associated with the connection, or <see langword="null"/> if the connection is not HTTP-based.</returns>
/// <remarks>
/// SignalR connections can run on top of HTTP transports like WebSockets or Long Polling, or other non-HTTP transports. As a result,
/// this method can sometimes return <see langword="null"/> depending on the configuration of your application.
/// </remarks>
Copy link
Member

@halter73 halter73 Feb 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a remark, beware that this won't show up in intellisense or the HttpConnectionContextExtensions generated docs. To see this you will have to got to the generated docs page dedicated exclusively to the GetHttpContext() method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think I'll put or <see langword="null" /> if there is no HTTP context in the summary (and leave this remark that adds more detail).

public static HttpContext GetHttpContext(this ConnectionContext connection)
{
return connection.Features.Get<IHttpContextFeature>()?.HttpContext;
Expand Down
4 changes: 4 additions & 0 deletions src/SignalR/common/SignalR.Common/src/HubException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ namespace Microsoft.AspNetCore.SignalR
/// <summary>
/// The exception thrown from a hub when an error occurs.
/// </summary>
/// <remarks>
/// Exceptions often contain sensitive information, such as connection information. Because of this, SignalR does not expose the details
/// of exceptions that occur on the server to the client. However, instances of <see cref="HubException"/> <b>are</b> sent to the client.
/// </remarks>
[Serializable]
public class HubException : Exception
{
Expand Down
3 changes: 3 additions & 0 deletions src/SignalR/server/Core/src/HubConnectionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

namespace Microsoft.AspNetCore.SignalR
{
/// <summary>
/// Encapsulates all information about an individual connection to a SignalR Hub.
/// </summary>
public class HubConnectionContext
{
private static readonly Action<object> _cancelReader = state => ((PipeReader)state).CancelPendingRead();
Expand Down