fix(ai-orchestration): observe agentStream output before draining#986
Open
tombeckenham wants to merge 1 commit into
Conversation
`executeAgent` awaited `drainAgentStream()` before `result.output`. When the stream failed, drain threw and the already-in-flight `output` promise was never observed, leaving a rejected promise unhandled. Under Node's default `--unhandled-rejections=throw` that terminates the worker — so an ordinary model error took down the process in addition to failing the step. Attach a no-op catch to `output` as soon as the result is recognized, then await it after draining. Adds the first coverage for `agentStream()`, asserting both that the step fails with `AgentStreamError` and that no unhandled rejection escapes.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tombeckenham
added
the
waiting-on: maintainer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Targets #979's branch, not
main.executeAgentawaiteddrainAgentStream()beforeresult.output. When the stream fails, drain throws and the already-in-flightoutputpromise is never observed — leaving a rejected promise unhandled. Under Node's default--unhandled-rejections=throwthat terminates the worker, so an ordinary model error took down the process in addition to correctly failing the step.Reproduced against the real
workflow-coreruntime before fixing:The fix attaches a no-op catch to
outputas soon as the result is recognized, then awaits it after draining. Behaviour on the success path is unchanged.Also adds the first test coverage for
agentStream()— it was exported fromindex.tsbut had no unit or E2E exercise, which is why this path went unnoticed. The test asserts both halves: the step fails withAgentStreamError, and no unhandled rejection escapes. Verified it fails without the source change.✅ Checklist
pnpm run test:pr.🚀 Release Impact
@tanstack/ai-orchestrationis still unreleased (0.0.0) and #979 already carries aminorchangeset introducing the package. This fixes code from that same unreleased changeset, so a second changeset would only add noise. Happy to add one if you'd rather it be itemised.