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

Fix typescript trace bugs #1275

Merged
merged 6 commits into from
Dec 30, 2024
Merged

Conversation

kyzyx
Copy link
Contributor

@kyzyx kyzyx commented Dec 29, 2024

Fixes #1274


Important

Fix TypeScript tracing bugs in BamlCtxManager by handling undefined responses and ensuring proper return values.

  • Behavior:
    • Fix span.finish in BamlCtxManager to handle undefined response by defaulting to an empty object.
    • Ensure ctx.run returns the result in traceFnSync and traceFnAsync methods of BamlCtxManager.

This description was created by Ellipsis for 8faf23e. It will automatically update as commits are pushed.

Copy link

vercel bot commented Dec 29, 2024

@kyzyx is attempting to deploy a commit to the Gloo Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 8faf23e in 8 seconds

More details
  • Looked at 31 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. engine/language_client_typescript/async_context_vars.js:54
  • Draft comment:
    The change from span.finish(response, manager); to span.finish(response === undefined ? {} : response, manager); ensures that the finish method is called with an empty object if response is undefined. This prevents potential errors when response is undefined. This change is appropriate and resolves part of the issue described.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The changes in the PR correctly address the issue described in the PR description and issue body. The modifications ensure that the traced functions return the expected values, resolving the errors mentioned.

Workflow ID: wflow_dp9ssR1zGvre0Uz6


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@aaronvg aaronvg enabled auto-merge December 30, 2024 21:56
@aaronvg aaronvg added this pull request to the merge queue Dec 30, 2024
Merged via the queue into BoundaryML:canary with commit e41b5aa Dec 30, 2024
6 of 7 checks passed
@aaronvg
Copy link
Contributor

aaronvg commented Dec 30, 2024

thanks for your contribution!

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.

Tracing in JS is broken
2 participants