-
Notifications
You must be signed in to change notification settings - Fork 0
feat: distributed tracing - 3 #10
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: eval-pr-25092-target-1763483260809
Are you sure you want to change the base?
feat: distributed tracing - 3 #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
@sanxroz Sorry, I encountered an error while trying to start the review. Please try again later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 5 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx">
<violation number="1" location="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx:328">
`t("trace_reference_id")` introduces a new translation key, but no locale entry exists for it, so the UI will render the raw key text instead of a localized label. Please add the string to the locale files when adding a new `t()` usage.</violation>
</file>
<file name="packages/lib/server/getServerErrorFromUnknown.ts">
<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:129">
String causes now return a plain `Error`, so `getServerErrorFromUnknown` can violate its contract and downstream code loses `statusCode`/`data`, breaking error normalization.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| {responseVercelIdHeader ?? ""} {t(messageKey, { date, count })} | ||
| {error.data?.traceId && ( | ||
| <div className="text-subtle mt-2 text-xs"> | ||
| <span className="font-medium">{t("trace_reference_id")}:</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t("trace_reference_id") introduces a new translation key, but no locale entry exists for it, so the UI will render the raw key text instead of a localized label. Please add the string to the locale files when adding a new t() usage.
Prompt for AI agents
Address the following comment on packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx at line 328:
<comment>`t("trace_reference_id")` introduces a new translation key, but no locale entry exists for it, so the UI will render the raw key text instead of a localized label. Please add the string to the locale files when adding a new `t()` usage.</comment>
<file context>
@@ -323,6 +323,12 @@ const getError = ({
{responseVercelIdHeader ?? ""} {t(messageKey, { date, count })}
+ {error.data?.traceId && (
+ <div className="text-subtle mt-2 text-xs">
+ <span className="font-medium">{t("trace_reference_id")}:</span>
+ <code className="ml-1 select-all break-all font-mono">{error.data.traceId}</code>
+ </div>
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
@sanxroz Sorry, I encountered an error while trying to start the review. Please try again later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 issues found across 5 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx">
<violation number="1" location="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx:328">
The new trace reference label calls an undefined translation key, so users will see the literal key text instead of a localized string. Add the entry to the locale files to keep the UI consistent.</violation>
</file>
<file name="packages/lib/server/getServerErrorFromUnknown.ts">
<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:79">
Spreading `prismaError.data` here throws whenever a traced Prisma error is processed because `getHttpError` never populates that property, so the error handler crashes instead of returning a response.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The hypothesis relies on the assumption that spreading an undefined value throws, but in JavaScript object spread treats `undefined`/`null` as a no-op (e.g., `{ ...undefined }` evaluates to `{}`). Therefore `traceId ? { ...tracedData, ...prismaError.data, traceId } : prismaError.data` cannot raise the claimed TypeError, so the violation is factually incorrect.</violation>
<violation number="2" location="packages/lib/server/getServerErrorFromUnknown.ts:91">
Trying to spread `stripeError.data` will crash runtime for traced Stripe errors because that property is undefined on the HttpError created by getHttpError.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Spreading `stripeError.data` cannot crash because JS object spread simply skips `undefined` sources. `{ ...undefined }` evaluates to `{}`, so the trace branch remains safe even when `stripeError.data` is undefined. The reported runtime error cannot occur, making the violation invalid.</violation>
<violation number="3" location="packages/lib/server/getServerErrorFromUnknown.ts:124">
Spreading `error.data` causes traced generic errors to throw because the HttpError returned by getHttpError never sets the `data` property.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The same incorrect premise appears here: spreading `error.data` does not throw when the value is undefined, since object spread ignores undefined sources. Hence traced generic errors will not crash, and the violation is false.</violation>
<violation number="4" location="packages/lib/server/getServerErrorFromUnknown.ts:129">
Returning a plain `Error` for string causes violates the function’s `HttpError` return contract, so downstream code loses `statusCode` and trace metadata and may fail.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| {responseVercelIdHeader ?? ""} {t(messageKey, { date, count })} | ||
| {error.data?.traceId && ( | ||
| <div className="text-subtle mt-2 text-xs"> | ||
| <span className="font-medium">{t("trace_reference_id")}:</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new trace reference label calls an undefined translation key, so users will see the literal key text instead of a localized string. Add the entry to the locale files to keep the UI consistent.
Prompt for AI agents
Address the following comment on packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx at line 328:
<comment>The new trace reference label calls an undefined translation key, so users will see the literal key text instead of a localized string. Add the entry to the locale files to keep the UI consistent.</comment>
<file context>
@@ -323,6 +323,12 @@ const getError = ({
{responseVercelIdHeader ?? ""} {t(messageKey, { date, count })}
+ {error.data?.traceId && (
+ <div className="text-subtle mt-2 text-xs">
+ <span className="font-medium">{t("trace_reference_id")}:</span>
+ <code className="ml-1 select-all break-all font-mono">{error.data.traceId}</code>
+ </div>
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
| statusCode: stripeError.statusCode, | ||
| message: stripeError.message, | ||
| cause: stripeError.cause, | ||
| data: traceId ? { ...tracedData, ...stripeError.data, traceId } : stripeError.data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to spread stripeError.data will crash runtime for traced Stripe errors because that property is undefined on the HttpError created by getHttpError.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Spreading stripeError.data cannot crash because JS object spread simply skips undefined sources. { ...undefined } evaluates to {}, so the trace branch remains safe even when stripeError.data is undefined. The reported runtime error cannot occur, making the violation invalid.
Prompt for AI agents
Address the following comment on packages/lib/server/getServerErrorFromUnknown.ts at line 91:
<comment>Trying to spread `stripeError.data` will crash runtime for traced Stripe errors because that property is undefined on the HttpError created by getHttpError.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Spreading `stripeError.data` cannot crash because JS object spread simply skips `undefined` sources. `{ ...undefined }` evaluates to `{}`, so the trace branch remains safe even when `stripeError.data` is undefined. The reported runtime error cannot occur, making the violation invalid.</comment>
<file context>
@@ -34,37 +38,71 @@ function parseZodErrorIssues(issues: ZodIssue[]): string {
+ statusCode: stripeError.statusCode,
+ message: stripeError.message,
+ cause: stripeError.cause,
+ data: traceId ? { ...tracedData, ...stripeError.data, traceId } : stripeError.data,
+ });
}
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| data: traceId ? { ...tracedData, ...stripeError.data, traceId } : stripeError.data, | |
| data: traceId ? { ...(tracedData ?? {}), ...(stripeError.data ?? {}), traceId } : stripeError.data, |
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
1 similar comment
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
2 similar comments
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 5 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx">
<violation number="1" location="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx:328">
The new label calls `t("trace_reference_id")`, but that translation key does not exist in the repo, so the UI will show the literal key instead of localized text. Define the string in the locale files or reuse an existing key before rendering it.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| {responseVercelIdHeader ?? ""} {t(messageKey, { date, count })} | ||
| {error.data?.traceId && ( | ||
| <div className="text-subtle mt-2 text-xs"> | ||
| <span className="font-medium">{t("trace_reference_id")}:</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new label calls t("trace_reference_id"), but that translation key does not exist in the repo, so the UI will show the literal key instead of localized text. Define the string in the locale files or reuse an existing key before rendering it.
Prompt for AI agents
Address the following comment on packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx at line 328:
<comment>The new label calls `t("trace_reference_id")`, but that translation key does not exist in the repo, so the UI will show the literal key instead of localized text. Define the string in the locale files or reuse an existing key before rendering it.</comment>
<file context>
@@ -323,6 +323,12 @@ const getError = ({
{responseVercelIdHeader ?? ""} {t(messageKey, { date, count })}
+ {error.data?.traceId && (
+ <div className="text-subtle mt-2 text-xs">
+ <span className="font-medium">{t("trace_reference_id")}:</span>
+ <code className="ml-1 select-all break-all font-mono">{error.data.traceId}</code>
+ </div>
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 5 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/lib/server/getServerErrorFromUnknown.ts">
<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:55">
`traceId` augmentation spreads optional objects like `tracedData`/`prismaError.data` without defaulting to `{}`, so traced errors without attached data will throw before an `HttpError` can be returned. Wrap each spread target in `?? {}` (or guard) before merging trace metadata.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Object spread syntax gracefully ignores `undefined`/`null` operands instead of throwing. Running this scenario in Node (`console.log({ ...undefined })`) produces `{}` without errors, so spreading `tracedData`, `prismaError.data`, etc. when they are undefined is safe. The reported TypeError cannot occur, making this claim incorrect.</violation>
<violation number="2" location="packages/lib/server/getServerErrorFromUnknown.ts:128">
The function `getServerErrorFromUnknown` violates its own return type signature. It is declared to return an `HttpError`, but for string inputs, it now returns a standard `Error`. This will cause runtime failures in any code that consumes this function and expects an `HttpError` object.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| return new HttpError({ | ||
| statusCode, | ||
| message: cause.message, | ||
| data: traceId ? { ...tracedData, traceId } : undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
traceId augmentation spreads optional objects like tracedData/prismaError.data without defaulting to {}, so traced errors without attached data will throw before an HttpError can be returned. Wrap each spread target in ?? {} (or guard) before merging trace metadata.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Object spread syntax gracefully ignores undefined/null operands instead of throwing. Running this scenario in Node (console.log({ ...undefined })) produces {} without errors, so spreading tracedData, prismaError.data, etc. when they are undefined is safe. The reported TypeError cannot occur, making this claim incorrect.
Prompt for AI agents
Address the following comment on packages/lib/server/getServerErrorFromUnknown.ts at line 55:
<comment>`traceId` augmentation spreads optional objects like `tracedData`/`prismaError.data` without defaulting to `{}`, so traced errors without attached data will throw before an `HttpError` can be returned. Wrap each spread target in `?? {}` (or guard) before merging trace metadata.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Object spread syntax gracefully ignores `undefined`/`null` operands instead of throwing. Running this scenario in Node (`console.log({ ...undefined })`) produces `{}` without errors, so spreading `tracedData`, `prismaError.data`, etc. when they are undefined is safe. The reported TypeError cannot occur, making this claim incorrect.</comment>
<file context>
@@ -34,37 +38,71 @@ function parseZodErrorIssues(issues: ZodIssue[]): string {
+ return new HttpError({
+ statusCode,
+ message: cause.message,
+ data: traceId ? { ...tracedData, traceId } : undefined,
+ });
+ }
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| data: traceId ? { ...tracedData, traceId } : undefined, | |
| data: traceId ? { ...(tracedData ?? {}), traceId } : undefined, |
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 5 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/lib/server/getServerErrorFromUnknown.ts">
<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:44">
The unwrapping logic for `TracedError` is not robust enough to handle nested instances. A single `if` statement is used, but a `while` loop would be required to recursively unwrap multiple layers of `TracedError`, preventing the `traceId` from being lost and ensuring the original error is correctly processed.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The code unwraps a `TracedError` once, but there is no evidence in the repository or documentation that `TracedError` instances can be nested. Without a demonstrated scenario where multiple layers occur, the suggestion to switch to a `while` loop is speculative; it may also be an intentional design choice. Given the lack of concrete impact, this is too uncertain to report.</violation>
<violation number="2" location="packages/lib/server/getServerErrorFromUnknown.ts:129">
String causes are no longer wrapped into an `HttpError`, so callers receive a plain `Error` without `statusCode` or trace metadata, breaking the function’s contract and downstream error handling.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| let traceId: string | undefined; | ||
| let tracedData: Record<string, unknown> | undefined; | ||
|
|
||
| if (cause instanceof TracedError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unwrapping logic for TracedError is not robust enough to handle nested instances. A single if statement is used, but a while loop would be required to recursively unwrap multiple layers of TracedError, preventing the traceId from being lost and ensuring the original error is correctly processed.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The code unwraps a TracedError once, but there is no evidence in the repository or documentation that TracedError instances can be nested. Without a demonstrated scenario where multiple layers occur, the suggestion to switch to a while loop is speculative; it may also be an intentional design choice. Given the lack of concrete impact, this is too uncertain to report.
Prompt for AI agents
Address the following comment on packages/lib/server/getServerErrorFromUnknown.ts at line 44:
<comment>The unwrapping logic for `TracedError` is not robust enough to handle nested instances. A single `if` statement is used, but a `while` loop would be required to recursively unwrap multiple layers of `TracedError`, preventing the `traceId` from being lost and ensuring the original error is correctly processed.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The code unwraps a `TracedError` once, but there is no evidence in the repository or documentation that `TracedError` instances can be nested. Without a demonstrated scenario where multiple layers occur, the suggestion to switch to a `while` loop is speculative; it may also be an intentional design choice. Given the lack of concrete impact, this is too uncertain to report.</comment>
<file context>
@@ -34,37 +38,71 @@ function parseZodErrorIssues(issues: ZodIssue[]): string {
+ let traceId: string | undefined;
+ let tracedData: Record<string, unknown> | undefined;
+
+ if (cause instanceof TracedError) {
+ traceId = cause.traceId;
+ tracedData = cause.data;
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: System Design Agent
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
1 similar comment
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 5 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/lib/server/getServerErrorFromUnknown.ts">
<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:129">
When the cause is a string you now return a plain Error instead of an HttpError, so downstream code loses statusCode/data and the function no longer conforms to its declared return type.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: typescriptUnderstanding.*</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
2 similar comments
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
This PR is on top of calcom#24861
This PR displays the trace reference id on the frontend
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Summary by cubic
Adds end-to-end tracing for errors and shows a trace reference in the booking UI. Improves server error normalization across TRPC, Zod, Prisma, and Stripe, with tests.
Written for commit a821001. Summary will update automatically on new commits.