Fix DateTime timezone mismatch causing Delay step to hang indefinitely with PostgreSQL#1425
Fix DateTime timezone mismatch causing Delay step to hang indefinitely with PostgreSQL#1425
Conversation
…tuck with PostgreSQL Agent-Logs-Url: https://github.com/danielgerlag/workflow-core/sessions/85da3a83-cc5a-4865-8002-bac682193a86 Co-authored-by: danielgerlag <2357007+danielgerlag@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes an Entity Framework (EF Core) persistence timezone mismatch where timestamp with time zone values from PostgreSQL can be materialized as DateTimeKind.Local, causing SleepUntil/Delay-related timestamps to be interpreted incorrectly and workflows to hang.
Changes:
- Added an
EnsureUtchelper to properly convertDateTimeKind.Localvalues to UTC viaToUniversalTime. - Replaced
DateTime.SpecifyKind(..., Utc)withEnsureUtc(...)for DateTime fields read from EF persistence models (WorkflowInstance,EventSubscription,Event).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static DateTime EnsureUtc(DateTime dateTime) | ||
| { | ||
| if (dateTime.Kind == DateTimeKind.Local) | ||
| return dateTime.ToUniversalTime(); | ||
| return DateTime.SpecifyKind(dateTime, DateTimeKind.Utc); | ||
| } |
There was a problem hiding this comment.
The new EnsureUtc behavior (converting DateTimeKind.Local via ToUniversalTime) isn’t covered by an automated regression test, so it could regress back to SpecifyKind without being detected—especially since most CI environments run with TimeZoneInfo.Local set to UTC. Consider adding a test (e.g., in the EF/PostgreSQL persistence tests) that forces a non-UTC local zone for the process (on Linux, setting TZ + calling TimeZoneInfo.ClearCachedData()), then verifies that a persisted SleepUntil coming back as Kind=Local is converted to the expected UTC instant.
| private static DateTime EnsureUtc(DateTime dateTime) | ||
| { | ||
| if (dateTime.Kind == DateTimeKind.Local) | ||
| return dateTime.ToUniversalTime(); |
There was a problem hiding this comment.
EnsureUtc currently calls DateTime.SpecifyKind(dateTime, DateTimeKind.Utc) for inputs that are already DateTimeKind.Utc. Returning the original value when dateTime.Kind == DateTimeKind.Utc would make the intent clearer and avoid an unnecessary copy.
| return dateTime.ToUniversalTime(); | |
| return dateTime.ToUniversalTime(); | |
| if (dateTime.Kind == DateTimeKind.Utc) | |
| return dateTime; |
|
@copilot please address feedback |
…Local/Utc/Unspecified DateTime conversion Agent-Logs-Url: https://github.com/danielgerlag/workflow-core/sessions/ffb23322-90eb-473d-a351-30db6c1ef619 Co-authored-by: danielgerlag <2357007+danielgerlag@users.noreply.github.com>
Done in commit
|
The ExternalTokenExpiry DateTime? field was missed in the initial fix and would still suffer from incorrect UTC handling when PostgreSQL returns DateTime values with Kind=Local. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When using PostgreSQL persistence (
timestamp with time zonecolumns) in non-UTC environments, EF Core returnsDateTimevalues withKind=Local. The prior code usedDateTime.SpecifyKind(..., DateTimeKind.Utc)which relabels the kind without converting the value, introducing a UTC offset into all persisted timestamps — causingSleepUntilto be read as e.g. 1 hour later than stored, and the Delay step to never resume.Describe your implementation or design
Added a private
EnsureUtchelper inExtensionMethods.csthat performs a proper conversion:The helper handles all three
DateTimeKindcases:Localvalues are properly converted to UTC viaToUniversalTime(),Utcvalues are returned as-is (avoiding an unnecessary copy), andUnspecifiedvalues are labelled as UTC viaSpecifyKind(preserving the previous behaviour for databases like SQLite and SQL Server).Applied
EnsureUtcto all DateTime fields read from the persistence layer:ToWorkflowInstance:CreateTime,CompleteTime,SleepUntil,StartTime,EndTimeToEventSubscription:SubscribeAsOfToEvent:EventTimeToPersistablewrite paths are unchanged — values are already UTC when written.Tests
Added
[assembly: InternalsVisibleTo("WorkflowCore.Tests.Sqlite")]to the EF persistence assembly and addedExtensionMethodsFixtureinWorkflowCore.Tests.Sqlitewith 4 unit tests that directly callToWorkflowInstance()with all threeDateTimeKindvariants (Local,Utc,Unspecified) forCreateTimeandSleepUntil. TheKind=Localtests compute the expected UTC value using.ToUniversalTime()at test time, making them timezone-agnostic and runnable in any CI environment. All 10 SQLite persistence tests pass (6 existing + 4 new).Breaking change
No. Behavior is corrected for non-UTC environments; UTC environments see no change.
Additional context
The
DateTimeKind.Unspecifiedcase (e.g., SQLite, SQL Server withouttimestamp with time zone) continues to behave exactly as before via theSpecifyKindfallback.