Handle TaskCanceledExceptions for all clients #6245
Conversation
|
Potential fix for #6165 |
| @@ -56,11 +57,8 @@ public ClientFactory() | |||
| } | |||
|
|
|||
| var creds = AzureSession.Instance.AuthenticationFactory.GetServiceClientCredentials(context, endpoint); | |||
| var newHandlers = GetCustomHandlers(); | |||
| TClient client = (newHandlers == null || newHandlers.Length == 0) | |||
There was a problem hiding this comment.
@markcowl why was this check for custom handlers removed?
There was a problem hiding this comment.
Discussed offline - because we move custom handler addition into the CreateCustomClient implementation later in this PR
|
|
||
| foreach (IClientAction action in GetActions()) | ||
| { | ||
| action.Apply<TClient>(client, profile, endpoint); | ||
| } | ||
|
|
||
| return client; | ||
|
|
There was a problem hiding this comment.
@markcowl nit: remove new line
| } | ||
| catch (TaskCanceledException) when (tries++ < MaxTries) | ||
| { | ||
| Thread.Sleep(WaitInterval); |
There was a problem hiding this comment.
@markcowl should we be using TestUtilities.Wait(WaitInterval) here?
| autoRestHandler.MaxTries = 0; | ||
| task = autorestClient.ResourceGroups.ListWithHttpMessagesAsync(); | ||
| Assert.Throws<TaskCanceledException>(() => task.ConfigureAwait(false).GetAwaiter().GetResult()); | ||
|
|
There was a problem hiding this comment.
@markcowl nit: remove these last few new lines
| task = autorestClient.ResourceGroups.ListWithHttpMessagesAsync(); | ||
| Assert.Throws<TaskCanceledException>(() => task.ConfigureAwait(false).GetAwaiter().GetResult()); | ||
|
|
||
|
|
There was a problem hiding this comment.
@markcowl do we need an assert for a successful retry of a list call and validating that content was returned from the call?
There was a problem hiding this comment.
I think we have that covered elsewhere
| void SleepIfRunningLive(TimeSpan timeSpan) | ||
| { | ||
| var testModeSetting = Environment.GetEnvironmentVariable(TestModeVariableKey); | ||
| if (string.IsNullOrWhiteSpace(testModeSetting) || !string.Equals(TestModeVariableValue, testModeSetting, StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
@markcowl this should check for Record and Live
Description
Checklist
CONTRIBUTING.mdplatyPSmodule