-
Notifications
You must be signed in to change notification settings - Fork 371
Improve key missing error handling with custom hooks #6521
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: main
Are you sure you want to change the base?
Improve key missing error handling with custom hooks #6521
Conversation
…ibility Co-authored-by: kevin <[email protected]>
Cursor Agent can help with this pull request. Just |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
@thiskevinwang can you share here or in private what is the intention with these new hooks and what do they unlock ? Are they aimed for "internal" aka Clerk usage or for developers/customers ?
In this supposed to be a recovery mechanism when Keyless fails to work, or is it overriding Keyless ? Does it unlock a new way to create keys ?
|
||
/** | ||
* Optional hook invoked when the publishable key is missing. If it returns a response, it will be used as the middleware result. | ||
* If it returns void, the request will continue unauthenticated. | ||
*/ | ||
onMissingPublishableKey?: ( | ||
request: NextRequest, | ||
) => NextMiddlewareReturn | Promise<NextMiddlewareReturn> | void | Promise<void>; | ||
|
||
/** | ||
* Optional hook invoked when the secret key is missing. If it returns a response, it will be used as the middleware result. | ||
* If it returns void, the request will continue unauthenticated. | ||
*/ | ||
onMissingSecretKey?: ( | ||
request: NextRequest, | ||
) => NextMiddlewareReturn | Promise<NextMiddlewareReturn> | void | Promise<void>; |
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.
If I understand the usage correctly those should only be executed in DEV. Also if we are thinking these hooks as something that will enable extra Clerk functionality we should prefix them with __internal_
.
// tsup.config.ts | ||
import { defineConfig } from 'tsup'; | ||
var tsup_config_default = defineConfig(() => { | ||
return { | ||
entry: { | ||
index: 'src/index.ts', | ||
}, | ||
minify: false, | ||
clean: true, | ||
sourcemap: true, | ||
format: ['cjs', 'esm'], | ||
legacyOutput: true, | ||
dts: true, | ||
}; | ||
}); | ||
export { tsup_config_default as default }; | ||
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsidHN1cC5jb25maWcudHMiXSwKICAic291cmNlc0NvbnRlbnQiOiBbImNvbnN0IF9faW5qZWN0ZWRfZmlsZW5hbWVfXyA9IFwiL3dvcmtzcGFjZS9wYWNrYWdlcy90eXBlcy90c3VwLmNvbmZpZy50c1wiO2NvbnN0IF9faW5qZWN0ZWRfZGlybmFtZV9fID0gXCIvd29ya3NwYWNlL3BhY2thZ2VzL3R5cGVzXCI7Y29uc3QgX19pbmplY3RlZF9pbXBvcnRfbWV0YV91cmxfXyA9IFwiZmlsZTovLy93b3Jrc3BhY2UvcGFja2FnZXMvdHlwZXMvdHN1cC5jb25maWcudHNcIjtpbXBvcnQgeyBkZWZpbmVDb25maWcgfSBmcm9tICd0c3VwJztcblxuZXhwb3J0IGRlZmF1bHQgZGVmaW5lQ29uZmlnKCgpID0+IHtcbiAgcmV0dXJuIHtcbiAgICBlbnRyeToge1xuICAgICAgaW5kZXg6ICdzcmMvaW5kZXgudHMnLFxuICAgIH0sXG4gICAgbWluaWZ5OiBmYWxzZSxcbiAgICBjbGVhbjogdHJ1ZSxcbiAgICBzb3VyY2VtYXA6IHRydWUsXG4gICAgZm9ybWF0OiBbJ2NqcycsICdlc20nXSxcbiAgICBsZWdhY3lPdXRwdXQ6IHRydWUsXG4gICAgZHRzOiB0cnVlLFxuICB9O1xufSk7XG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQXlOLFNBQVMsb0JBQW9CO0FBRXRQLElBQU8sc0JBQVEsYUFBYSxNQUFNO0FBQ2hDLFNBQU87QUFBQSxJQUNMLE9BQU87QUFBQSxNQUNMLE9BQU87QUFBQSxJQUNUO0FBQUEsSUFDQSxRQUFRO0FBQUEsSUFDUixPQUFPO0FBQUEsSUFDUCxXQUFXO0FBQUEsSUFDWCxRQUFRLENBQUMsT0FBTyxLQUFLO0FBQUEsSUFDckIsY0FBYztBQUFBLElBQ2QsS0FBSztBQUFBLEVBQ1A7QUFDRixDQUFDOyIsCiAgIm5hbWVzIjogW10KfQo= |
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.
was this autogenerated ?
@@ -57,7 +57,7 @@ export async function createOrReadKeylessAction(): Promise<null | Omit<Accountle | |||
const result = await import('../server/keyless-node.js').then(m => m.createOrReadKeyless()).catch(() => null); | |||
|
|||
if (!result) { | |||
errorThrower.throwMissingPublishableKeyError(); | |||
// In non-keyless scenarios, allow continuing without throwing so the app can render. |
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.
❓ Why is Keyless being affected by these changes ?
// Fall through to allow request to continue unauthenticated | ||
const res = NextResponse.next(); | ||
setRequestHeadersOnNextResponse(res, request, { | ||
[constants.Headers.AuthStatus]: 'signed-out', | ||
}); | ||
return res; | ||
} |
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.
Can you clarify how this operates on the client ?
@clerk/clerk-react
will throw if the publishable key is missing, since to the url encoded withing the PK is required to hotload clerk-js
.
Description
This PR updates the Clerk Next.js SDK to provide more flexible handling for missing
PUBLISHABLE_KEY
andSECRET_KEY
environment variables. Previously, the SDK would throw disruptive runtime errors, blocking Next.js app rendering.Changes introduced:
clerkMiddleware
:onMissingPublishableKey
andonMissingSecretKey
options toClerkMiddlewareOptions
.NextResponse
, it is used as the middleware result.void
, the middleware proceeds by returningNextResponse.next()
and settingx-clerk-auth-status: signed-out
, allowing the app to render without crashing.packages/nextjs/src/app-router/keyless-actions.ts
for a missing publishable key during keyless operations has been removed. It now returnsnull
, allowing the application to continue gracefully.How to test:
Configure the new hooks in your
clerkMiddleware
to observe the custom behavior whenNEXT_PUBLIC_CLERK_PUBLISHABLE_KEY
orCLERK_SECRET_KEY
are missing from your environment.If these hooks are not provided, the existing throwing behavior will remain.
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Slack Thread