Skip to content

Comments

Fix ArgumentOutOfRangeException for extreme datetime#5347

Open
rbans96 wants to merge 2 commits intomainfrom
personal/ribans/fix-date-error
Open

Fix ArgumentOutOfRangeException for extreme datetime#5347
rbans96 wants to merge 2 commits intomainfrom
personal/ribans/fix-date-error

Conversation

@rbans96
Copy link
Contributor

@rbans96 rbans96 commented Jan 23, 2026

Description

This change is to fix the error if customer provides birthdate as extreme datetime

"PreciseTimeStamp": 2026-01-21T12:26:56.1684910Z,
"body": {StatusCode} error returned,
"env_ex_msg": The added or subtracted value results in an un-representable DateTime. (Parameter 'value'),
"env_ex_type": System.ArgumentOutOfRangeException,
"stackTrace": at System.DateTime.ThrowDateArithmetic(Int32 param)
at Microsoft.Health.Fhir.Core.Features.Search.Expressions.SearchValueExpressionBuilderHelper.Microsoft.Health.Fhir.Core.Features.Search.SearchValues.ISearchValueVisitor.Visit(DateTimeSearchValue dateTime) in //src/Microsoft.Health.Fhir.Core/Features/Search/Expressions/Parsers/SearchValueExpressionBuilderHelper.cs:line 106
at Microsoft.Health.Fhir.Core.Features.Search.Expressions.Parsers.SearchParameterExpressionParser.Parse(SearchParameterInfo searchParameter, SearchModifier modifier, String value) in /
/src/Microsoft.Health.Fhir.Core/Features/Search/Expressions/Parsers/SearchParameterExpressionParser.cs:line 52
at Microsoft.Health.Fhir.Core.Features.Search.Expressions.Parsers.ExpressionParser.ParseImpl(String[] resourceTypes, ReadOnlySpan1 key, String value) in /_/src/Microsoft.Health.Fhir.Core/Features/Search/Expressions/Parsers/ExpressionParser.cs:line 295 at Microsoft.Health.Fhir.Core.Features.Search.SearchOptionsFactory.<>c__DisplayClass14_0.<Create>b__6(Tuple2 q) in //src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs:line 413
at System.Linq.Enumerable.SelectListIterator2.MoveNext() at System.Linq.Enumerable.WhereEnumerableIterator1.MoveNext()
at System.Collections.Generic.List1.AddRange(IEnumerable1 collection)
at Microsoft.Health.Fhir.Core.Features.Search.SearchOptionsFactory.Create(String compartmentType, String compartmentId, String resourceType, IReadOnlyList1 queryParameters, Boolean isAsyncOperation, Boolean useSmartCompartmentDefinition, ResourceVersionType resourceVersionTypes, Boolean onlyIds, Boolean isIncludesOperation) in /_/src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs:line 408 at Microsoft.Health.Fhir.Core.Features.Search.SearchService.SearchAsync(String resourceType, IReadOnlyList1 queryParameters, CancellationToken cancellationToken, Boolean isAsyncOperation, ResourceVersionType resourceVersionTypes, Boolean onlyIds, Boolean isIncludesOperation) in /
/src/Microsoft.Health.Fhir.Core/Features/Search/SearchService.cs:line 59

Related issues

Addresses https://microsofthealth.visualstudio.com/Health/_workitems/edit/181592

Testing

Added E2E tets

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@rbans96 rbans96 added this to the CY25Q3/2Wk08 milestone Jan 23, 2026
@rbans96 rbans96 requested a review from a team as a code owner January 23, 2026 20:42
@rbans96 rbans96 added Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs No-ADR ADR not needed Bug Bug bug bug. labels Jan 23, 2026
Comment on lines +264 to +267
catch (Exception e)
{
Assert.Fail($"An unexpected '{e.GetType()}' was raised. The server should handle extreme approximate dates gracefully. Url: {Client.HttpClient.BaseAddress}. Error: {e.Message}");
}

Check notice

Code scanning / CodeQL

Generic catch clause Note test

Generic catch clause.

Copilot Autofix

AI about 1 month ago

General approach: Replace the generic catch (Exception e) with either more specific exception types or remove it altogether so that truly unexpected exceptions propagate naturally. Since we already handle FhirClientException explicitly and everything else is “unexpected,” the cleanest fix is to let the test framework report unexpected exceptions without catching them, thereby eliminating the generic catch clause.

Best concrete fix here: Remove the catch (Exception e) block from GivenAnApproximateDateSearchParamWithExtremeDate_WhenSearched_ThenServerShouldNotCrash. This preserves the desired behavior around FhirClientException (checking that the status code is not 500) while allowing any other unexpected exceptions to surface directly with full stack traces. No new imports or helpers are needed, and existing functionality is not materially changed: the test still fails when an unexpected exception occurs, but now via the framework instead of a manual Assert.Fail.

Specifically, in test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/DateSearchTests.cs, within the second test method shown (GivenAnApproximateDateSearchParamWithExtremeDate_WhenSearched_ThenServerShouldNotCrash), delete lines 264–267 containing the generic catch (Exception e) and the associated Assert.Fail call. Leave the rest of the method, including the try block and catch (FhirClientException fce), unchanged.


Suggested changeset 1
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/DateSearchTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/DateSearchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/DateSearchTests.cs
--- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/DateSearchTests.cs
+++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/DateSearchTests.cs
@@ -261,10 +261,6 @@
                     fce.StatusCode != HttpStatusCode.InternalServerError,
                     $"Server returned '{HttpStatusCode.InternalServerError}' for a valid approximate date search. This indicates an unhandled exception (overflow) in the date calculation logic. Url: {Client.HttpClient.BaseAddress}. Activity Id: {fce.Response.GetRequestId()}. Error: {fce.Message}");
             }
-            catch (Exception e)
-            {
-                Assert.Fail($"An unexpected '{e.GetType()}' was raised. The server should handle extreme approximate dates gracefully. Url: {Client.HttpClient.BaseAddress}. Error: {e.Message}");
-            }
         }
     }
 }
EOF
@@ -261,10 +261,6 @@
fce.StatusCode != HttpStatusCode.InternalServerError,
$"Server returned '{HttpStatusCode.InternalServerError}' for a valid approximate date search. This indicates an unhandled exception (overflow) in the date calculation logic. Url: {Client.HttpClient.BaseAddress}. Activity Id: {fce.Response.GetRequestId()}. Error: {fce.Message}");
}
catch (Exception e)
{
Assert.Fail($"An unexpected '{e.GetType()}' was raised. The server should handle extreme approximate dates gracefully. Url: {Client.HttpClient.BaseAddress}. Error: {e.Message}");
}
}
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
checked
{
long rawDifference = nowTicks - maxTicks;
differenceTicks = (long)(rawDifference * ApproximateMultiplier);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused how this could ever overflow. nowTicks is approximately known to be 6.37e17 and maxTicks can't be greater than roughly 1/3 long.maxValue or less than 0, so their difference can't be outside the range of long (~9.22e18). ApproximateMultiplier is less than one so differenceTicks is less than rawDifference.
Is there something about this calculation I'm missing?

{
// The server should NOT return 500 Internal Server Error for valid approximate date searches
// It's acceptable to return BadRequest if the server decides the date is out of reasonable bounds
Assert.True(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this test should have two valid scenarios, it should either expect success or a bad request.

@LTA-Thinking
Copy link
Contributor

I know we discussed this a few weeks ago, but I think the correct behavior here is to succeed and use the max or min datetime value. This should fix current 500s and not add new 400s for customers. If this returned 400 there wouldn't be a way to use dates close to the cutoff for approximate searches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug. No-ADR ADR not needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants