feat(test-utils): Add a way to wait for single spans for Span streaming#18986
feat(test-utils): Add a way to wait for single spans for Span streaming#18986JPeer264 merged 1 commit intolms/feat-span-streaming-pocfrom
Conversation
![cursor[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| for (const envelopeItem of envelopeItems) { | ||
| if (!isSpanV2EnvelopeItem(envelopeItem)) { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Early return skips valid Span V2 items in envelope
Medium Severity
The waitForSpanV2 function returns false immediately when encountering any non-Span V2 envelope item, rather than skipping it and continuing to check other items. If an envelope contains mixed item types (e.g., an event item followed by a Span V2 item), the function will never find valid Span V2 spans because it exits on the first non-matching item. The logic differs from waitForSpansV2, which correctly uses an if (isSpanV2EnvelopeItem) pattern to skip non-Span V2 items.
There was a problem hiding this comment.
I don't think it can be mixed
size-limit report 📦
|
|
|
||
| for (const envelopeItem of envelopeItems) { | ||
| if (!isSpanV2EnvelopeItem(envelopeItem)) { | ||
| return false |
There was a problem hiding this comment.
should we continue here?
| return false | |
| continue; |
There was a problem hiding this comment.
That would be this comment: #18986 (comment)
Not sure if it is possible to mix envelope items
|
I'll merge. I think when having more tests these functions might change a little (but maybe they stay as is). I'm happy to do follow ups ones there are issues with this |
JPeer264
merged commit f528c45
into
lms/feat-span-streaming-poc
2 participants
How it should be used is in the JSDoc. It worked quite well for my Cloudflare tests
Closes #18987 (added automatically)