Skip to content

Fix backend error message details #2421

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

Merged

Conversation

Nikpolik
Copy link
Contributor

@Nikpolik Nikpolik commented Dec 20, 2023

Description

This was caused by API errors being parsed twice. Once in the response error handling code and again when initializing a new ClerkAPIResponseError.

This is was fixed in v5 with the removal of ClerkAPIResponseError.

ClerkAPIResponseError is also used by clerk-js the change was to the backend package.

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/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore

@Nikpolik Nikpolik self-assigned this Dec 20, 2023
@Nikpolik Nikpolik requested a review from a team as a code owner December 20, 2023 15:33
@Nikpolik Nikpolik requested review from desiprisg and removed request for a team December 20, 2023 15:33
Copy link

changeset-bot bot commented Dec 20, 2023

🦋 Changeset detected

Latest commit: d98ed12

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

This PR includes changesets to release 6 packages
Name Type
@clerk/backend Patch
@clerk/fastify Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/remix Patch
@clerk/clerk-sdk-node 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 nikpolik/v4-fix-backend-error-message-details branch from cc48c63 to ff516a8 Compare December 20, 2023 15:39
@panteliselef panteliselef removed the request for review from desiprisg December 20, 2023 17:09
@@ -154,7 +154,7 @@ export function buildRequest(options: CreateBackendApiOptions) {
data: deserialize(data),
errors: null,
};
} catch (err) {
} catch (err: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
} catch (err: any) {
} catch (err: unknown) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was done on purpose. Since the result is passed to with withLegacyReturn all type safety is gone here. I can fix the types as well but wanted to get the fix out quick.

@Nikpolik Nikpolik force-pushed the nikpolik/v4-fix-backend-error-message-details branch 2 times, most recently from 6561462 to 6d3958e Compare December 21, 2023 15:39
This was caused by API errors beeing parsed twice. Once in the response
error handling and again when initializing a new ClerkAPIResponseError.
@Nikpolik Nikpolik force-pushed the nikpolik/v4-fix-backend-error-message-details branch from 6d3958e to d98ed12 Compare December 21, 2023 15:43
@Nikpolik
Copy link
Contributor Author

Nikpolik commented Dec 21, 2023

@SokratisVidros I updated the fix a bit to make it simpler, also added a test for this case (its present in v5)

@nikosdouvlis nikosdouvlis merged commit 71b4b9c into release/v4 Jan 8, 2024
@nikosdouvlis nikosdouvlis deleted the nikpolik/v4-fix-backend-error-message-details branch January 8, 2024 10:24
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.

4 participants