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

Include clerkTraceId for backend api errors #1894

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

Nikpolik
Copy link
Member

@Nikpolik Nikpolik commented Oct 16, 2023

Description

This PR adds clerkTraceId in ClerkBackendApiResponse. This can help debug issues with API responses especially when errors happen server side or transiently and they can't be viewed in the console.

Changes

  1. Use ClerkAPIResponseError from @clerk/shared.
  2. Updated ClerkBackendApiResponse interface to include clerkTraceId when available.
  3. Extract clerkTraceId either from backend error response or headers in buildRequest.
  4. Pass the new clerkTraceId value to withLegacyReturn and then to the constructor of ClerkAPIResponseError.
  5. Update toString for ClerkAPIResponseError so it can also log the clerkTraceId.

Now callining .toString() on an error should show the trace id

Screenshot 2023-10-17 at 12 45 50 PM

The same is true for simply calling console.log on the error

Screenshot 2023-10-16 at 4 49 20 PM

Notes

DX is a bit weird here since fields that are different between success and error cannot be accessed by default in typescript and they do not appear in code completion(intelisense/language server). They can be accessed however by
checking if they exist in the object.

if ("clerkTraceId" in clerkBackendApiResponse) {
  console.log(`Error Trace: ${response.clerkTraceId}`);
}

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Packages affected

  • @clerk/clerk-js
  • @clerk/clerk-react
  • @clerk/nextjs
  • @clerk/remix
  • @clerk/types
  • @clerk/themes
  • @clerk/localizations
  • @clerk/clerk-expo
  • @clerk/backend
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/fastify
  • @clerk/chrome-extension
  • gatsby-plugin-clerk
  • build/tooling/chore

@changeset-bot
Copy link

changeset-bot bot commented Oct 16, 2023

🦋 Changeset detected

Latest commit: 1c04f6c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@clerk/backend Patch
@clerk/shared Patch
@clerk/fastify Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
@clerk/clerk-js Patch
@clerk/clerk-expo Patch
@clerk/clerk-react Patch
@clerk/chrome-extension Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Nikpolik Nikpolik force-pushed the plt-155-add-traceid-to-backend-errors branch 2 times, most recently from c1305f8 to 5f4fade Compare October 17, 2023 08:54
@Nikpolik Nikpolik marked this pull request as ready for review October 17, 2023 09:34
@Nikpolik Nikpolik requested a review from a team as a code owner October 17, 2023 09:34
@Nikpolik Nikpolik self-assigned this Oct 17, 2023
packages/backend/src/api/request.ts Outdated Show resolved Hide resolved
packages/backend/src/api/request.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@georgepsarakis georgepsarakis left a comment

Choose a reason for hiding this comment

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

Nice work @Nikpolik 👏 !

packages/backend/src/api/request.ts Outdated Show resolved Hide resolved
.changeset/sour-comics-stare.md Outdated Show resolved Hide resolved
.changeset/sour-comics-stare.md Outdated Show resolved Hide resolved
packages/backend/src/api/request.ts Show resolved Hide resolved
packages/shared/src/errors/Error.ts Outdated Show resolved Hide resolved
packages/backend/src/api/request.ts Outdated Show resolved Hide resolved
@dimkl dimkl requested a review from SokratisVidros October 19, 2023 09:09
@Nikpolik Nikpolik force-pushed the plt-155-add-traceid-to-backend-errors branch 3 times, most recently from c5d6fc0 to 83f200a Compare October 20, 2023 11:58
@Nikpolik Nikpolik force-pushed the plt-155-add-traceid-to-backend-errors branch from 83f200a to 99d6b0e Compare October 26, 2023 11:21
@Nikpolik Nikpolik changed the base branch from main to main-v4 October 26, 2023 11:23
packages/backend/src/api/request.ts Show resolved Hide resolved
packages/backend/src/api/request.ts Outdated Show resolved Hide resolved
packages/backend/src/api/request.ts Outdated Show resolved Hide resolved
@Nikpolik Nikpolik force-pushed the plt-155-add-traceid-to-backend-errors branch 2 times, most recently from 7dfdf84 to 16b44aa Compare October 27, 2023 13:44
clerkTraceId is used when available and defaults to cloudflares CF Ray id
when its missing.
@Nikpolik Nikpolik force-pushed the plt-155-add-traceid-to-backend-errors branch from 16b44aa to 1c04f6c Compare October 30, 2023 11:40
@Nikpolik Nikpolik added this pull request to the merge queue Oct 30, 2023
Merged via the queue into main-v4 with commit bc19fe0 Oct 30, 2023
12 checks passed
@Nikpolik Nikpolik deleted the plt-155-add-traceid-to-backend-errors branch October 30, 2023 13:26
@dimkl
Copy link
Contributor

dimkl commented Oct 30, 2023

@Nikpolik Could you also cherry-pick this change into main?

octoper pushed a commit that referenced this pull request Oct 31, 2023
…1894)

clerkTraceId is used when available and defaults to cloudflares CF Ray id
when its missing.
@Nikpolik Nikpolik restored the plt-155-add-traceid-to-backend-errors branch October 31, 2023 13:50
@dimkl dimkl deleted the plt-155-add-traceid-to-backend-errors branch October 31, 2023 13:55
@Nikpolik Nikpolik restored the plt-155-add-traceid-to-backend-errors branch October 31, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants