add configurable timeout #1103
add configurable timeout #1103
Conversation
| @@ -15,9 +15,9 @@ namespace Microsoft.Azure.SignalR | |||
| { | |||
| internal class ServiceRouteHelper | |||
| { | |||
| public static async Task RedirectToService(HttpContext context, string hubName, IList<IAuthorizeData> authorizationData) | |||
| public static async Task RedirectToService(HttpContext context, Type hubType, IList<IAuthorizeData> authorizationData) | |||
There was a problem hiding this comment.
|
|
||
| namespace Microsoft.Azure.SignalR | ||
| { | ||
| internal interface INegotiateHandler |
There was a problem hiding this comment.
| if (handshakeTimeout.Value.CompareTo(TimeSpan.Zero) <= 0 || | ||
| handshakeTimeout.Value.CompareTo(Constants.Periods.MaxCustomHandshakeTimeout) > 0) | ||
| { | ||
| Log.FailToSetCustomHandshakeTimeout(logger, new ArgumentOutOfRangeException(nameof(handshakeTimeout))); |
There was a problem hiding this comment.
move the check to some validation when startup? otherwise customer gets such log for every request
| @@ -66,7 +66,7 @@ private Endpoint CreateNegotiateEndpoint(RouteEndpoint routeEndpoint) | |||
| // Replaces the negotiate endpoint with one that does the service redirect | |||
| var routeEndpointBuilder = new RouteEndpointBuilder(async context => | |||
| { | |||
| await ServiceRouteHelper.RedirectToService(context, hubMetadata.HubType.Name, null); | |||
| await ServiceRouteHelper.RedirectToService(context, hubMetadata.HubType, null); | |||
There was a problem hiding this comment.
make CreateNegotiateEndpoint a generic method then? It only runs when start, while your approach creates the type for every request.
There was a problem hiding this comment.
The CreateNegotiateEndpoint is used here, there's still no way to get the <THub> here.
The ApplyAsync is invoked on each negotiate request, while CreateNegotiateEndpoint needs the CandidateSet as a input parameter.
How about I change the ServiceRouteHelper to ServiceRouteHelper<Thub>, and DI to the gloabl service collection, and then get the service here and call the ServiceRouteHelper<THub>.RedirectToService() method? Then I don't need INegotiateHandler and Type of the hub any more.
wanlwanl
force-pushed
the
wanl/configurable-handshake-timeout
branch
from
042f46b to
0b3e8d6
Compare
| { | ||
| return type => | ||
| { | ||
| var methodInfo = typeof(NegotiateMatcherPolicy).GetMethod(nameof(CreateNegotiateEndpointCore), BindingFlags.NonPublic | BindingFlags.Instance); |
There was a problem hiding this comment.
move it to outside of lamda, otherwise it is evaluated everytime.
| private Endpoint CreateNegotiateEndpoint(RouteEndpoint routeEndpoint) | ||
| private Func<Type, Endpoint> CreateNegotiateEndpoint(Type hubType, RouteEndpoint routeEndpoint) | ||
| { | ||
| var methodInfo = typeof(NegotiateMatcherPolicy).GetMethod(nameof(CreateNegotiateEndpointCore), BindingFlags.NonPublic | BindingFlags.Static); |
wanlwanl
force-pushed
the
wanl/configurable-handshake-timeout
branch
from
eb6a389 to
a06216d
Compare
Summary of the changes (Less than 80 chars)
Fixes #bugnumber (in this specific format)