fix(core): Fix error on langgraph resume#19354
fix(core): Fix error on langgraph resume#19354jadengis wants to merge 2 commits intogetsentry:developfrom
Conversation
nicohrubec
left a comment
There was a problem hiding this comment.
Thanks for contributing! Looks good to me, but we'll need to add some tests before merging. Should we take over?
|
@nicohrubec Feel free to take over! I would really appreciate the help. This has been burning me in production. |
|
@nicohrubec Actually nvm I added a test and verified that it fails without the proposed fix. Let me know if this is acceptable. 🙏 |
nicohrubec
left a comment
There was a problem hiding this comment.
I'd rather have us test against the real API in this case. We have node integration tests to do that.
Specifically I would just add a graph.invoke(null) to this file:
https://github.com/getsentry/sentry-javascript/blob/develop/dev-packages/node-integration-tests/suites/tracing/langgraph/scenario.mjs
Then we need to update the test.ts in the same folder to check for the spans. Don't think we need the unit tests in this case
|
@jadengis Merged the fix! You are added as a co-author and mentioned in the changelog so I hope that's all good. Fix will go out with the next release |
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Closes #19353