-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(medusa): API workflow subscription #13886
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: develop
Are you sure you want to change the base?
Conversation
…d]/[transaction_id]/[step_id] directory
…tions_{workflow_id}_{transaction_id}_{step_id}_subscribe.yaml
…rkflows-executions_{workflow_id}_{transaction_id}_{step_id}_subscribe directory
🦋 Changeset detectedLatest commit: c9137d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 74 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@v0eak is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
Refactor fetch handling to use a promise and async generator for streaming response.
add js-sdk to changeset
|
Also modified the js-sdk to return before it receives its first body response, because otherwise, when a stream is established, its not possible to abort it until data is sent. in the case of workflow subscriptions, this might take a while. If we then have a component that would like to unmount its stream, it is unable to, because it has not yet received its first data. const { stream, abort } = await sdk.client.fetchStream("/admin/stream")
setAbortStream(() => abort)abortStream will not be set until the await fetchStream resolves. |
Summary
What —
Change req.query to req.params in appropriate api routes
to /api/admin/workflows-executions/[workflow_id]/[transaction_id]/subscribe/route.ts, because it is not possible to subscribe to individual steps, only to individual workflows.
Why — Current implementation relies on req.query to get the id of workflow/transaction, but it should get them from req.params instead
How — Change req.query to req.params
Testing — https://docs.medusajs.com/api/admin#workflows-executions_getworkflowsexecutionsworkflow_idsubscribe
Examples
https://docs.medusajs.com/resources/js-sdk#stream-server-sent-events
// Example usageChecklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
Add any additional context, related issues, or references that might help the reviewer understand this PR.
Note
Switch admin workflow subscription to path params and a single-execution endpoint; update JS SDK fetchStream to always yield a stream and throw on non-OK, with docs aligned.
req.paramsinstead ofreq.queryinadmin/workflows-executions/[workflow_id]/[transaction_id]/subscribeand[workflow_id]/subscriberoutes..../[workflow_id]/[transaction_id]/subscribe.client.fetchStreamto return a non-null async generator; defer fetch, throwHttpErroron non-OK, and stream viaeventswith abort support.FetchStreamResponsetype:streamis non-null.streamand handleHttpErrorby callingabort.@medusajs/medusa: minor;@medusajs/js-sdk: patch.Written by Cursor Bugbot for commit c9137d9. This will update automatically on new commits. Configure here.