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

Fix issue I was experiencing in my time zone #1650

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

IEvangelist
Copy link
Contributor

@IEvangelist IEvangelist commented Sep 27, 2023

The issue or feature being addressed

When comparing DateTime and DateTimeOffset exceptions can be thrown during implicit conversion. When I was testing the circuit breaker strategy within my .NET 8 rtm pipeline, I was seeing the following exception:

Exception Details

System.ArgumentOutOfRangeException: 'The UTC time represented when the offset is applied must be between year 0 and 10,000. (Parameter 'offset')'
at System.DateTimeOffset.ValidateDate(DateTime dateTime, TimeSpan offset)
at System.DateTimeOffset..ctor(DateTime dateTime)
at System.DateTimeOffset.op_Implicit(DateTime dateTime)
at Polly.CircuitBreaker.CircuitStateController1.IsDateTimeOverflow(DateTimeOffset utcNow, TimeSpan breakDuration) at Polly.CircuitBreaker.CircuitStateController1.OpenCircuitFor_NeedsLock(Outcome1 outcome, TimeSpan breakDuration, Boolean manual, ResilienceContext context, Task& scheduledTask) at Polly.CircuitBreaker.CircuitStateController1.OpenCircuit_NeedsLock(Outcome1 outcome, Boolean manual, ResilienceContext context, Task& scheduledTask) at Polly.CircuitBreaker.CircuitStateController1.OnActionFailureAsync(Outcome1 outcome, ResilienceContext context) at Polly.CircuitBreaker.CircuitBreakerResilienceStrategy1.d__51.MoveNext() at System.Threading.Tasks.ValueTask1.get_Result()
at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable1.ConfiguredValueTaskAwaiter.GetResult() at Polly.Utils.Pipeline.BridgeComponent1.<g__ConvertValueTaskAsync|5_0>d1.MoveNext() at System.Threading.Tasks.ValueTask1.get_Result()
at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable1.ConfiguredValueTaskAwaiter.GetResult() at Polly.Utils.Pipeline.CompositeComponent.<ExecuteCoreWithTelemetry>d__132.MoveNext()
at System.Threading.Tasks.ValueTask1.get_Result() at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable1.ConfiguredValueTaskAwaiter.GetResult()
at Polly.ResiliencePipeline.d__2`1.MoveNext()
at Program.<

$>d__0.MoveNext() in C:\Users\dapine\source\repos\dotnet-docs\docs\core\resilience\snippets\resilience\Program.cs:line 70
at Program.(String[] args)

This PR includes fixes to a failing unit test and addresses the issue in the source as well.

image

Special shoutout to @stephentoub for his help with this bug fix.

Details on the issue fix or feature implementation

Confirm the following

  • I started this PR by branching from the head of the default branch
  • I have targeted the PR to merge into the default branch
  • I have included unit tests for the issue/feature
  • I have successfully run a local build

I'm target the dotnet-8 branch, and that's what I started from.

@martintmk martintmk merged commit 232b2f8 into App-vNext:dotnet-8 Sep 27, 2023
2 checks passed
@martintmk
Copy link
Contributor

Possibly also fix for the main branch?

Cc @martincostello

@martincostello
Copy link
Member

This does look like something we need to fix in main before we ship Polly 8.0.0 too (our .NET 8 branch will likely ship as 8.1.0 after .NET 8 RTMs).

@IEvangelist can you replicate this there too?

@martincostello
Copy link
Member

It's OK I checked and it's something in main too - I've cherry picked the fix and something to detect such patterns again in the future in #1651.

@joelhulen
Copy link
Member

Great catch, @IEvangelist! Thanks for the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants