refactor(core): improve user messages with clearer context#189
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughReplaces all legacy error classes ( ChangesAura Error System Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/core/src/shared/utils.ts (1)
93-98:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnresolved provider env placeholders still become a live Basic header.
Lines 94-95 fall back to the raw arguments when the env lookup misses. In
packages/core/src/oauth/notion.ts, Lines 56-61, that turns absentNOTION_CLIENT_ID/NOTION_CLIENT_SECRETvars into literal credentials, so Line 97 never throws and the misconfiguration is only discovered after an outbound token request. This helper needs an explicit “resolve env key” vs “use literal credential” contract, and the fail-fast path should use a configuration-oriented Aura code instead ofAUTH_BASIC_CREDENTIALS_INVALID.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/shared/utils.ts` around lines 93 - 98, The helper createBasicAuthHeader currently falls back to the raw username/password when getEnv returns undefined (getEnv(username) ?? username), which lets unresolved env placeholders become live credentials; change the contract to require resolved env values: call getEnv(username) and getEnv(password) and if either returns undefined/null, do NOT use the literal argument — throw a configuration-oriented error (replace AuraAuthError with a config error class such as AuraConfigError) with a clear config-missing code (e.g., "CONFIG_ENV_VAR_MISSING") and message identifying which env key failed; ensure createBasicAuthHeader, getEnv usage, and the thrown error class/name are updated so callers like oauth/notion.ts fail fast on missing envs.packages/core/src/api/signOut.ts (1)
60-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix failure response contract (
redirectURL) and return an error status.Line 63 uses
redirectsURL(typo), and this failuretoResponsecurrently returns default 200. That breaks response shape consistency and can mask failures at HTTP level.Suggested fix
toResponse: () => { - return Response.json({ - success: false, - redirect: false, - redirectsURL: null, - }) + return Response.json( + { + success: false, + redirect: false, + redirectURL: null, + error: { code, message }, + }, + { headers, status: 400 } + ) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/api/signOut.ts` around lines 60 - 64, The failure response in signOut.ts returns the wrong field name and a 200 status; update the Response.json call in the signOut (or its toResponse) failure branch to use the correct redirectURL property (not redirectsURL) and return a non-200 HTTP status (e.g., status: 500 or other appropriate error code) so the response shape and status indicate failure; ensure the payload still contains success: false and include an error message or null redirectURL as before.packages/core/src/api/credentials.ts (1)
80-85:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNarrow the invalid-credentials branch to the explicit credentials error code.
Line 80 currently treats any
AuraAuthErroras invalid credentials. That mislabels unrelated auth/config/origin errors asINVALID_CREDENTIALSand returns the same 401 path.Suggested fix
- if (isAuraAuthError(error)) { + if (isAuraAuthError(error) && error.code === "AUTH_CREDENTIALS_INVALID") { logger?.log("INVALID_CREDENTIALS", { severity: "warning", structuredData: { path: "/signIn/credentials" }, }) return invalidCredentials }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/api/credentials.ts` around lines 80 - 85, The code currently treats any AuraAuthError as invalid credentials; change the condition so the invalid-credentials branch only triggers for the explicit credential error code (e.g., check error.code or error.errorCode equals the project's credential error constant) instead of any AuraAuthError. Concretely, update the if around isAuraAuthError(error) to something like isAuraAuthError(error) && error.code === <CREDENTIALS_ERROR_CODE>, then call logger?.log("INVALID_CREDENTIALS", ...) and return invalidCredentials; otherwise let other AuraAuthError cases fall through or be handled separately.packages/core/src/actions/callback/callback.ts (1)
41-53:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't report provider-declared OAuth errors as "missing parameters".
This branch runs when the callback contains an OAuth
errorpayload from the provider, e.g.access_deniedorserver_error. ThrowingAUTH_CALLBACK_MISSING_PARAMETERSturns an explicit upstream denial/failure into the wrong message and removes the distinction between "provider rejected the flow" and "the callback was malformed." Use a dedicated AuraAuthError code for authorization error responses instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/actions/callback/callback.ts` around lines 41 - 53, The code currently detects an OAuth provider error via OAuthAuthorizationErrorResponse.safeParse and then throws AuraAuthError with code "AUTH_CALLBACK_MISSING_PARAMETERS"; change this to throw a distinct authorization error that preserves provider details (e.g., throw new AuraAuthError({ code: "AUTH_CALLBACK_AUTHORIZATION_ERROR", meta: { error, error_description } })) so provider-declared errors (access_denied, server_error) are not misreported as missing parameters; update the thrown error in the same branch where OAuthAuthorizationErrorResponse, criticalAuthErrors, and logger are used and ensure the logger still records severity and structuredData.packages/core/src/actions/callback/access-token.ts (1)
25-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't collapse every missing precondition into
INVALID_OAUTH_PROVIDER_URL_CONFIG.This guard also fires when
clientSecret,code, orcodeVerifieris missing, so callers will get a provider-URL error for failures that are unrelated to the provider URL. Split the URL-only validation from the general callback/config preconditions and map them to different catalog codes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/actions/callback/access-token.ts` around lines 25 - 36, Split the single guard into two checks: first validate the provider-URL related fields (e.g., redirectURI and any provider URL-specific input) and if those are missing log structuredData and throw AuraAuthError with code "INVALID_OAUTH_PROVIDER_URL_CONFIG"; then validate the remaining callback/config preconditions (clientId, clientSecret, code, codeVerifier, accessToken) and if any are missing log structuredData and throw a different AuraAuthError code such as "INVALID_OAUTH_CONFIGURATION". Keep the same logger structuredData keys (has_client_id, has_client_secret, has_access_token, has_redirect_uri, has_code, has_code_verifier) and apply them to both checks so the logs show which specific fields are absent; update the throw sites in access-token.ts accordingly.
🧹 Nitpick comments (1)
packages/core/test/actions/callback/access-token.test.ts (1)
66-68: ⚡ Quick winThese tests are over-coupled to mutable internal prose; assert stable error contract fields instead.
packages/core/test/actions/callback/access-token.test.ts#L66-L68: assert errorcode/type(or status+payload) rather than long internal message text.packages/core/test/actions/callback/access-token.test.ts#L97-L99: same refactor—prefer contract fields over prose.packages/core/test/actions/callback/access-token.test.ts#L135-L137: same refactor—replace exact full sentence assertion with stable identifiers.packages/core/test/actions/callback/userinfo.test.ts#L117-L119: switch to checking stable error identifiers.packages/core/test/actions/callback/userinfo.test.ts#L150-L152: switch to checking stable error identifiers.packages/core/test/actions/callback/userinfo.test.ts#L177-L179: switch to checking stable error identifiers.packages/core/test/actions/callback/userinfo.test.ts#L258-L260: switch to checking stable error identifiers.packages/core/test/actions/signIn/authorization.test.ts#L110-L112: prefer code/type assertions for rejection paths.packages/core/test/actions/signIn/authorization.test.ts#L151-L153: same refactor to stable contract assertions.packages/core/test/actions/signIn/authorization.test.ts#L165-L167: same refactor to stable contract assertions.packages/core/test/oauth.test.ts#L27-L29: assert stable structured error attributes instead of exact message.packages/core/test/oauth.test.ts#L54-L56: assert stable structured error attributes instead of exact message.packages/core/test/api/signOut.test.ts#L22-L24: assert stable error code/type for missing session token path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/test/actions/callback/access-token.test.ts` around lines 66 - 68, Replace fragile assertions that match full error message prose with assertions on stable error contract fields (e.g., error.code, error.type, or status+payload) in the listed tests: packages/core/test/actions/callback/access-token.test.ts (the rejects.toThrow at L66-L68, and similar at L97-L99 and L135-L137), packages/core/test/actions/callback/userinfo.test.ts (L117-L119, L150-L152, L177-L179, L258-L260), packages/core/test/actions/signIn/authorization.test.ts (L110-L112, L151-L153, L165-L167), packages/core/test/oauth.test.ts (L27-L29, L54-L56), and packages/core/test/api/signOut.test.ts (L22-L24); locate the Promise rejection checks (e.g., the .rejects.toThrow calls) and change them to assert the error object has the expected stable fields (like expect(err.code).toBe(...) or expect(err.type).toBe(...) or expect(err.status).toBe(...) and/or inspect err.payload) instead of matching the full human-readable message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/actions/callback/access-token.ts`:
- Around line 70-94: The JSON.parse step (response.json()) needs its own
try/catch so parse failures are classified as response-format errors rather than
generic transport errors: wrap the response.json() call in a small try block and
if it throws, log via logger and throw an AuraAuthError with code
"INVALID_OAUTH_ACCESS_TOKEN_RES_FORMAT" in access-token.ts (affecting the logic
around OAuthAccessTokenResponse/OAuthAccessTokenErrorResponse handling), and
similarly throw "INVALID_OAUTH_USER_INFO_RES_FORMAT" in userinfo.ts (around the
OAuth user info response validation). Keep the outer try to handle
transport/network failures and preserve existing isAuraAuthError rethrows and
other error codes; only reclassify JSON parse exceptions to the new *_RES_FORMAT
codes and include the original parse error as the cause when creating the
AuraAuthError.
In `@packages/core/src/actions/signIn/authorization-url.ts`:
- Around line 23-27: The code currently constructs the provider URL with new
URL(baseURL) which can throw a native TypeError for malformed strings; wrap the
URL construction in a try/catch around the new URL(baseURL) call in the
authorization URL logic (the spot using authorizeConfig/baseURL and creating
url) and on any exception rethrow an AuraAuthError with code
"INVALID_OAUTH_PROVIDER_URL_CONFIG" (include the original error message/details
in the AuraAuthError payload or log) so malformed provider URLs surface as the
standardized AuraAuthError instead of leaking a native TypeError.
In `@packages/core/src/actions/signUp/signUp.ts`:
- Line 10: The fallback empty Zod schema uses z.object() which is not the
intended API in Zod v4; update the fallback in the sign-up action so that body:
config?.schema ?? z.object({}) uses an explicit empty shape. Locate the
occurrence of config?.schema and replace the z.object() fallback with
z.object({}) (e.g., in the signUp handler where body is assigned) to ensure a
proper empty-object schema.
In `@packages/core/src/api/signIn.ts`:
- Around line 77-80: When projecting AuraAuthError into API payloads, replace
uses of the internal error.message with the user-facing error.userMessage:
inside the isAuraAuthError(error) branches (e.g., in signIn.ts where code =
error.code and message = error.message), set message = error.userMessage; make
the same change in the corresponding Aura error branches in credentials.ts,
signOut.ts, signUp.ts, and updateSession.ts so each branch uses
error.userMessage when building the response object (preserve existing code =
error.code behavior and only swap the message source).
In `@packages/core/src/cookie.ts`:
- Around line 104-106: The current check in cookie handling only tests `if
(!cookies)` which misses empty arrays; update the condition where `cookies` is
validated (refer to the `cookies` variable and the throw of `AuraAuthError` with
code `"SET_COOKIE_NOT_FOUND"`) to treat an empty collection as "not found" as
well (e.g., check for `cookies` falsy OR `cookies.length === 0`) so that empty
responses trigger the `SET_COOKIE_NOT_FOUND` error instead of
`SET_COOKIE_INVALID_VALUE`.
In `@packages/core/src/shared/crypto.ts`:
- Around line 83-85: In verifyCSRF, the catch currently replaces every error
with AuraAuthError({ code: "CSRF_TOKEN_MISSING", cause: error }), which hides
explicit CSRF_TOKEN_MISMATCH errors; change the catch to rethrow the original
error when it's already an AuraAuthError (or when error.code ===
"CSRF_TOKEN_MISMATCH"), and only wrap/throw a new AuraAuthError with code
"CSRF_TOKEN_MISSING" for other unexpected failures—refer to the verifyCSRF
function and AuraAuthError usage to implement this conditional rethrowing logic
so mismatch errors are preserved.
In `@packages/core/src/shared/unstable_error.ts`:
- Around line 407-413: The CONFIG_BASE_URL_MISSING catalog entry currently has
empty message and userMessage which causes AuraAuthError.message to be blank and
toResponse() to emit an invalid empty-message response; update the
CONFIG_BASE_URL_MISSING object (the constant with key CONFIG_BASE_URL_MISSING)
to provide non-empty defaults for both message and userMessage (e.g. a concise
internal message and a user-facing string), keeping the type, statusCode, and
name unchanged so that AuraAuthError.message, toResponse(), and errorHandler.ts
produce a valid response (this same change should also be applied at the other
occurrence referenced around lines 729-733).
In `@packages/core/src/validator/registry.ts`:
- Around line 61-62: The three schema-type fallback throws that currently use
throw new AuraAuthError({ code: "SCHEMA_INVALID_MODE" }) are misclassifying
unsupported schema fallthroughs; replace those error instances so they throw
AuraAuthError with code "SCHEMA_UNSUPPORTED" instead (or alternatively add an
explicit runtime mode guard earlier if you intend to validate mode), i.e.,
locate each occurrence of throw new AuraAuthError({ code: "SCHEMA_INVALID_MODE"
}) in registry.ts (the schema-type fallback branches) and change the code value
to "SCHEMA_UNSUPPORTED".
---
Outside diff comments:
In `@packages/core/src/actions/callback/access-token.ts`:
- Around line 25-36: Split the single guard into two checks: first validate the
provider-URL related fields (e.g., redirectURI and any provider URL-specific
input) and if those are missing log structuredData and throw AuraAuthError with
code "INVALID_OAUTH_PROVIDER_URL_CONFIG"; then validate the remaining
callback/config preconditions (clientId, clientSecret, code, codeVerifier,
accessToken) and if any are missing log structuredData and throw a different
AuraAuthError code such as "INVALID_OAUTH_CONFIGURATION". Keep the same logger
structuredData keys (has_client_id, has_client_secret, has_access_token,
has_redirect_uri, has_code, has_code_verifier) and apply them to both checks so
the logs show which specific fields are absent; update the throw sites in
access-token.ts accordingly.
In `@packages/core/src/actions/callback/callback.ts`:
- Around line 41-53: The code currently detects an OAuth provider error via
OAuthAuthorizationErrorResponse.safeParse and then throws AuraAuthError with
code "AUTH_CALLBACK_MISSING_PARAMETERS"; change this to throw a distinct
authorization error that preserves provider details (e.g., throw new
AuraAuthError({ code: "AUTH_CALLBACK_AUTHORIZATION_ERROR", meta: { error,
error_description } })) so provider-declared errors (access_denied,
server_error) are not misreported as missing parameters; update the thrown error
in the same branch where OAuthAuthorizationErrorResponse, criticalAuthErrors,
and logger are used and ensure the logger still records severity and
structuredData.
In `@packages/core/src/api/credentials.ts`:
- Around line 80-85: The code currently treats any AuraAuthError as invalid
credentials; change the condition so the invalid-credentials branch only
triggers for the explicit credential error code (e.g., check error.code or
error.errorCode equals the project's credential error constant) instead of any
AuraAuthError. Concretely, update the if around isAuraAuthError(error) to
something like isAuraAuthError(error) && error.code ===
<CREDENTIALS_ERROR_CODE>, then call logger?.log("INVALID_CREDENTIALS", ...) and
return invalidCredentials; otherwise let other AuraAuthError cases fall through
or be handled separately.
In `@packages/core/src/api/signOut.ts`:
- Around line 60-64: The failure response in signOut.ts returns the wrong field
name and a 200 status; update the Response.json call in the signOut (or its
toResponse) failure branch to use the correct redirectURL property (not
redirectsURL) and return a non-200 HTTP status (e.g., status: 500 or other
appropriate error code) so the response shape and status indicate failure;
ensure the payload still contains success: false and include an error message or
null redirectURL as before.
In `@packages/core/src/shared/utils.ts`:
- Around line 93-98: The helper createBasicAuthHeader currently falls back to
the raw username/password when getEnv returns undefined (getEnv(username) ??
username), which lets unresolved env placeholders become live credentials;
change the contract to require resolved env values: call getEnv(username) and
getEnv(password) and if either returns undefined/null, do NOT use the literal
argument — throw a configuration-oriented error (replace AuraAuthError with a
config error class such as AuraConfigError) with a clear config-missing code
(e.g., "CONFIG_ENV_VAR_MISSING") and message identifying which env key failed;
ensure createBasicAuthHeader, getEnv usage, and the thrown error class/name are
updated so callers like oauth/notion.ts fail fast on missing envs.
---
Nitpick comments:
In `@packages/core/test/actions/callback/access-token.test.ts`:
- Around line 66-68: Replace fragile assertions that match full error message
prose with assertions on stable error contract fields (e.g., error.code,
error.type, or status+payload) in the listed tests:
packages/core/test/actions/callback/access-token.test.ts (the rejects.toThrow at
L66-L68, and similar at L97-L99 and L135-L137),
packages/core/test/actions/callback/userinfo.test.ts (L117-L119, L150-L152,
L177-L179, L258-L260), packages/core/test/actions/signIn/authorization.test.ts
(L110-L112, L151-L153, L165-L167), packages/core/test/oauth.test.ts (L27-L29,
L54-L56), and packages/core/test/api/signOut.test.ts (L22-L24); locate the
Promise rejection checks (e.g., the .rejects.toThrow calls) and change them to
assert the error object has the expected stable fields (like
expect(err.code).toBe(...) or expect(err.type).toBe(...) or
expect(err.status).toBe(...) and/or inspect err.payload) instead of matching the
full human-readable message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 070c010d-9450-450a-bd03-d54f09121a37
📒 Files selected for processing (46)
packages/core/deno.jsonpackages/core/src/actions/callback/access-token.tspackages/core/src/actions/callback/callback.tspackages/core/src/actions/callback/userinfo.tspackages/core/src/actions/signIn/authorization-url.tspackages/core/src/actions/signIn/authorization.tspackages/core/src/actions/signUp/signUp.tspackages/core/src/api/credentials.tspackages/core/src/api/signIn.tspackages/core/src/api/signOut.tspackages/core/src/api/signUp.tspackages/core/src/api/updateSession.tspackages/core/src/client/client.tspackages/core/src/cookie.tspackages/core/src/jose.tspackages/core/src/oauth/index.tspackages/core/src/router/errorHandler.tspackages/core/src/session/jose-manager.tspackages/core/src/session/stateless.tspackages/core/src/session/strategy.tspackages/core/src/shared/crypto.tspackages/core/src/shared/errors.tspackages/core/src/shared/unstable_error.tspackages/core/src/shared/utils.tspackages/core/src/validator/registry.tspackages/core/src/validator/validator.tspackages/core/test/actions/callback/access-token.test.tspackages/core/test/actions/callback/callback.test.tspackages/core/test/actions/callback/userinfo.test.tspackages/core/test/actions/signIn/authorization.test.tspackages/core/test/actions/signIn/signIn.test.tspackages/core/test/actions/signOut/signOut.test.tspackages/core/test/api/signIn.test.tspackages/core/test/api/signInCredentials.test.tspackages/core/test/api/signOut.test.tspackages/core/test/api/signUp.test.tspackages/core/test/api/updateSession.test.tspackages/core/test/instance.test.tspackages/core/test/jose.test.tspackages/core/test/oauth.test.tspackages/elysia/package.jsonpackages/express/package.jsonpackages/hono/package.jsonpackages/next/package.jsonpackages/react-router/package.jsonpackages/react/package.json
💤 Files with no reviewable changes (1)
- packages/core/src/shared/errors.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/api/signOut.ts`:
- Around line 60-65: In the error response payload returned by the toResponse()
method in the signOut.ts file, change the key name from `redirectsURL` to
`redirectURL` to maintain consistency with the API contract used elsewhere in
the file. This single character fix (removing the 's' from 'redirects') ensures
the response structure matches the expected contract.
In `@packages/core/test/actions/signUp/signUp.test.ts`:
- Around line 93-95: Remove the debug artifacts from the test code in the
signUp.test.ts file. The temporary variable named `idk` and the associated
`console.log("idk: ", idk)` statement should be removed. Instead, directly parse
and assert on the response JSON by replacing the `idk` variable reference with
the actual `response.json()` call in the expect assertion to clean up the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0875489-b70f-4828-a017-f0a8c2c9a394
📒 Files selected for processing (20)
packages/core/src/actions/callback/access-token.tspackages/core/src/actions/signIn/authorization-url.tspackages/core/src/actions/signUp/signUp.tspackages/core/src/api/credentials.tspackages/core/src/api/signIn.tspackages/core/src/api/signOut.tspackages/core/src/api/signUp.tspackages/core/src/api/updateSession.tspackages/core/src/cookie.tspackages/core/src/router/errorHandler.tspackages/core/src/shared/crypto.tspackages/core/src/shared/unstable_error.tspackages/core/src/validator/registry.tspackages/core/test/actions/callback/access-token.test.tspackages/core/test/actions/signIn/signIn.test.tspackages/core/test/actions/signUp/signUp.test.tspackages/core/test/api/signIn.test.tspackages/core/test/api/signInCredentials.test.tspackages/core/test/api/signUp.test.tspackages/core/test/api/updateSession.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/test/api/signUp.test.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/core/test/api/signIn.test.ts
- packages/core/src/api/signIn.ts
- packages/core/src/api/updateSession.ts
- packages/core/src/actions/signIn/authorization-url.ts
- packages/core/src/shared/crypto.ts
- packages/core/src/api/signUp.ts
- packages/core/src/cookie.ts
- packages/core/src/router/errorHandler.ts
- packages/core/src/api/credentials.ts
- packages/core/test/api/signInCredentials.test.ts
- packages/core/src/shared/unstable_error.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/actions/callback/callback.ts (1)
22-32:⚠️ Potential issue | 🟠 MajorFix Zod v4 error message API usage.
In Zod v4, error messages must be passed via a configuration object with an
errorproperty, not as positional arguments. Update:
z.string("message")→z.string({ error: "message" })z.enum(array, "message")→z.enum(array, { error: "message" })The current code at lines 23-25 and lines 30-31 uses the deprecated v3 API and will not produce the intended error messages in Zod v4.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/actions/callback/callback.ts` around lines 22 - 32, Update the Zod schema definitions in the callback validation to use Zod v4's error message API. In the params object, change the z.enum call to pass the error message via a configuration object with an error property instead of as a positional argument. Similarly, update both z.string calls in the searchParams object to use the new syntax by passing an object with an error property containing the message instead of passing the message as a positional argument. This ensures the error messages are properly recognized in Zod v4.
🧹 Nitpick comments (1)
packages/core/src/actions/callback/callback.ts (1)
62-76: 💤 Low valueState mismatch returns JSON instead of throwing
AuraAuthError.This block returns
Response.json(...)directly rather than throwingAuraAuthError({ code: "AUTH_MISMATCHING_STATE" }). This is inconsistent with the rest of the error system refactor where security/protocol errors throwAuraAuthErrorand let the error handler serialize the response.Consider throwing
AuraAuthErrorfor consistency, though this approach works since the response needs custom headers to clear cookies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/actions/callback/callback.ts` around lines 62 - 76, The state mismatch check in the timingSafeEqual block is returning Response.json directly instead of throwing AuraAuthError, which is inconsistent with the error refactor pattern. Replace the Response.json return statement with throwing AuraAuthError using code "AUTH_MISMATCHING_STATE". Since the error also needs to clear cookies via clearCookieHeaders, attach the headers to the thrown AuraAuthError instance so the error handler can properly serialize the response with the required headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/core/src/actions/callback/callback.ts`:
- Around line 22-32: Update the Zod schema definitions in the callback
validation to use Zod v4's error message API. In the params object, change the
z.enum call to pass the error message via a configuration object with an error
property instead of as a positional argument. Similarly, update both z.string
calls in the searchParams object to use the new syntax by passing an object with
an error property containing the message instead of passing the message as a
positional argument. This ensures the error messages are properly recognized in
Zod v4.
---
Nitpick comments:
In `@packages/core/src/actions/callback/callback.ts`:
- Around line 62-76: The state mismatch check in the timingSafeEqual block is
returning Response.json directly instead of throwing AuraAuthError, which is
inconsistent with the error refactor pattern. Replace the Response.json return
statement with throwing AuraAuthError using code "AUTH_MISMATCHING_STATE". Since
the error also needs to clear cookies via clearCookieHeaders, attach the headers
to the thrown AuraAuthError instance so the error handler can properly serialize
the response with the required headers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3ff6d87-c26f-4bc2-80ea-a24fa2f63f2d
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockdeno.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
apps/nuxt/package.jsonpackages/core/deno.jsonpackages/core/package.jsonpackages/core/src/actions/callback/access-token.tspackages/core/src/actions/callback/callback.tspackages/core/src/actions/callback/userinfo.tspackages/core/src/actions/signIn/authorization-url.tspackages/core/src/actions/signIn/authorization.tspackages/core/src/api/credentials.tspackages/core/src/api/signIn.tspackages/core/src/api/signOut.tspackages/core/src/api/signUp.tspackages/core/src/api/updateSession.tspackages/core/src/client/client.tspackages/core/src/cookie.tspackages/core/src/jose.tspackages/core/src/oauth/index.tspackages/core/src/router/errorHandler.tspackages/core/src/session/jose-manager.tspackages/core/src/session/stateless.tspackages/core/src/session/strategy.tspackages/core/src/shared/crypto.tspackages/core/src/shared/errors.tspackages/core/src/shared/utils.tspackages/core/src/validator/registry.tspackages/core/src/validator/validator.tspackages/core/test/actions/callback/callback.test.tspackages/core/test/actions/signIn/signIn.test.tspackages/core/test/actions/signUp/signUp.test.ts
💤 Files with no reviewable changes (3)
- apps/nuxt/package.json
- packages/core/src/shared/errors.ts
- packages/core/src/validator/validator.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/package.json
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/core/src/session/jose-manager.ts
- packages/core/src/api/signOut.ts
- packages/core/src/api/signIn.ts
- packages/core/src/session/strategy.ts
- packages/core/src/api/updateSession.ts
- packages/core/src/actions/signIn/authorization-url.ts
- packages/core/src/api/signUp.ts
- packages/core/src/client/client.ts
- packages/core/src/shared/crypto.ts
- packages/core/src/session/stateless.ts
- packages/core/src/actions/callback/userinfo.ts
- packages/core/src/actions/signIn/authorization.ts
- packages/core/src/shared/utils.ts
- packages/core/src/oauth/index.ts
- packages/core/src/actions/callback/access-token.ts
- packages/core/src/api/credentials.ts
- packages/core/src/validator/registry.ts
- packages/core/src/jose.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request refactors and standardizes error handling across the authentication flows in
@aura-stack/auth.The changes improve error reporting by providing clearer user-facing messages and more structured diagnostic information for developers. All authentication errors now extend the
AuraAuthErrorbase class, creating a consistent error model throughout the library.The new error system includes structured metadata such as:
typecodemessageuserMessageThis information enables applications to present meaningful messages to end users while also providing developers with the context needed to diagnose and resolve issues.
As part of this work,
@aura-stack/routerhas been upgraded to v0.9.0, which provides the underlying error-handling infrastructure required for this refactor.Key Changes
AuraAuthErrorbase class.type,code,message, anduserMessage).@aura-stack/routerto v0.9.0 to support the new error architecture.Summary by CodeRabbit
Release Notes
Bug Fixes
Chores
@aura-stack/routerdependency to^0.9.0.0.7.2.