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

Refactor internal steps, headers and body creation #87

Merged
merged 15 commits into from
Apr 4, 2025

Conversation

CahidArda
Copy link
Collaborator

Previously, we had a single method for getting headers, getHeaders. This would compute the headers for wait for event, context.call etc. It would handle flow control and retries. It would handle failure function. This made it possible to keep all header related logic in a single place but it was complicated.

When it comes to request bodies, some of the logic was in getHeaders (timeoutHeaders), some of it was in auto-executor, some of it was in serve-many (invokeWorkflow).

With this PR:

  • we move the body and header calculations to the step functions themselves. There is a new headers.ts file which has most of the common logic for creating headers. Step specific headers are in steps.ts
  • we remove the submitStepsToQStash method from auto executor. There is a new qstash/submit-steps.ts to submit plan steps and result steps.

private prefixHeaders(contentType: string): HeadersResponse {
const { rawHeaders, workflowHeaders, failureHeaders } = this.headers;

const isCall = this.stepInfo?.lazyStep.stepType === "Call";
Copy link
Contributor

Choose a reason for hiding this comment

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

Please double check if the following conditions are equivalent in all cases:
Old version:
!step?.callUrl
New version:
this.stepInfo?.lazyStep.stepType === "Call"
this.stepInfo?.lazyStep instanceof LazyCallStep

[WORKFLOW_ID_HEADER]: this.workflowConfig.workflowRunId,
[WORKFLOW_URL_HEADER]: this.workflowConfig.workflowUrl,
[WORKFLOW_FEATURE_HEADER]: "LazyFetch,InitialBody",
[WORKFLOW_PROTOCOL_VERSION_HEADER]: WORKFLOW_PROTOCOL_VERSION,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

send in workflowHeaders

we were getting type errors after @types/express v5.0.1 was released a week ago. Pinning the version of this package in the workflow-js and examples/express to 5.0.0 fixed the issue. But this wasn't a good solution in the long term.

another solution was removing the dependency from examples/express and keeping it in workflow-js. We don't import anything from @types/express but we still need it in our dependencies.
@CahidArda CahidArda merged commit f868cf1 into main Apr 4, 2025
18 of 20 checks passed
@CahidArda CahidArda deleted the refactor-step-header-and-body branch April 4, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants