-
Notifications
You must be signed in to change notification settings - Fork 105
fix(core): add date parsing for resumeAt inside runtime
#343
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
base: main
Are you sure you want to change the base?
Conversation
|
|
@hrynevychroman is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Suggestion:
When events are deserialized from the backend, the resumeAt date field is returned as a string, but sleep.ts assigns this directly to waitItem.resumeAt which expects a Date object. This causes a TypeError later when runtime.ts tries to call .getTime() on the string value.
View Details
📝 Patch Details
diff --git a/packages/core/src/workflow/sleep.ts b/packages/core/src/workflow/sleep.ts
index 0b16fd3..1edbf9f 100644
--- a/packages/core/src/workflow/sleep.ts
+++ b/packages/core/src/workflow/sleep.ts
@@ -44,7 +44,9 @@ export function createSleep(ctx: WorkflowOrchestratorContext) {
) as WaitInvocationQueueItem | undefined;
if (waitItem) {
waitItem.hasCreatedEvent = true;
- waitItem.resumeAt = event.eventData.resumeAt;
+ waitItem.resumeAt = event.eventData.resumeAt
+ ? new Date(event.eventData.resumeAt)
+ : event.eventData.resumeAt;
}
return EventConsumerResult.Consumed;
}
Analysis
Type mismatch: resumeAt string assigned to Date-typed field causes runtime error
What fails: When a wait_created event is received in sleep.ts line 47, the resumeAt field from the deserialized event data (which is a string from JSON) is directly assigned to waitItem.resumeAt (which is typed as Date). This causes a TypeError: resumeAt.getTime is not a function later when runtime.ts line 487 tries to call .getTime() on the value.
How to reproduce:
- Create a workflow that calls
sleep() - The workflow gets suspended and creates a
wait_createdevent - The event is fetched from the backend and deserialized (date fields become strings in JSON)
- When the
wait_createdevent callback executes insleep.ts, it assigns the string towaitItem.resumeAt - On next execution,
runtime.tsline 487 attemptsqueueItem.resumeAt.getTime()and throws
Result: TypeError: resumeAt.getTime is not a function when processing wait queue items
Expected: The resumeAt field should always be a Date object, matching the type contract defined in WaitInvocationQueueItem (global.ts line 19)
Root cause: When events are deserialized from backend storage (JSON), date strings remain as strings rather than being automatically converted to Date objects. The fix in runtime.ts lines 343-345 converts these strings to Dates for runtime processing, but the same conversion was missing in sleep.ts where the deserialized event data is assigned to the queue item.
Fix applied: Wrapped the assignment in sleep.ts line 47-49 with a new Date() conversion, matching the pattern used in runtime.ts lines 343-345.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment below.
I do think we also really need to include e2e tests for the postgres world. That would've prevented an issue like this
| for (const event of events) { | ||
| if (event.eventType === 'wait_created') { | ||
| const resumeAt = event.eventData.resumeAt as Date; | ||
| const resumeAt = event.eventData.resumeAt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the world should return resumeAt as a date - so if this is working in vercel and local worlds, this this looks like a postgres world implementation issue. The real fix is postgres world should match the interface and return a date
The zod return type for resumeAt is already meant to be a date:
https://github.com/vercel/workflow/blob/main/packages/world/src/events.ts#L82-L82
|
fyi I've added postgres world e2e tests now in #356 It seems we have a couple of postgres e2e tests that are failing on different workbenches that need work: |

We receive this date as
stringso we need to parse itFix #340