Skip to content
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

[release/6.0] Use TimeSpan.FromMilliseconds for clientTimeoutInterval in ClientTime… #39325

Merged
merged 2 commits into from Jan 7, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 5, 2022

Backport of #39318 to release/6.0

/cc @BrennanConroy @campersau

Description

In 6.0 we refactored some code with timeouts and didn't update a log message so it now displays the wrong timeout value.

Fixes #39171

Customer Impact

When looking at logs for why a connection closed, you will see the incorrect value for how long a timeout occurred. For example, a 90 second timeout will be logged as 9 milliseconds.

Customer reported issue at #39171

Regression?

  • Yes
  • No

Regressed from 5.0 to 6.0

Risk

  • High
  • Medium
  • Low

Just changing the units of a value in a log message.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-signalr Includes: SignalR clients and servers label Jan 5, 2022
@ghost ghost added this to the 6.0.x milestone Jan 5, 2022
@ghost ghost added this to In Progress in Servicing Jan 5, 2022
@ghost
Copy link

ghost commented Jan 5, 2022

Hi @github-actions[bot]. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Jan 5, 2022
@ghost
Copy link

ghost commented Jan 5, 2022

Hi @github-actions[bot]. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 6, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.2 Jan 6, 2022
@dougbu
Copy link
Member

dougbu commented Jan 7, 2022

@BrennanConroy is this PR waiting for something❔

@BrennanConroy
Copy link
Member

Yes, I didn't know it was servicing approved until just now (you don't get notified when a label or milestone is added/changed).

@halter73 could you give this a quick review?

@halter73
Copy link
Member

halter73 commented Jan 7, 2022

We should probably rename these variables to include Milliseconds instead of Ticks in main.

@dougbu
Copy link
Member

dougbu commented Jan 7, 2022

Build won't pass until 1ES provider is fixed and I get #39364 in. Will need to repeat validation. So, I canceled the current build.

@dougbu
Copy link
Member

dougbu commented Jan 7, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@dougbu
Copy link
Member

dougbu commented Jan 7, 2022

All set for squish and munge @BrennanConroy

@BrennanConroy
Copy link
Member

Yes

@dougbu dougbu merged commit 1272293 into release/6.0 Jan 7, 2022
Servicing automation moved this from In Progress to Done Jan 7, 2022
@dougbu dougbu deleted the backport/pr-39318-to-release/6.0 branch January 7, 2022 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers Servicing-approved Shiproom has approved the issue
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants