refactor(core): improve user messages with clearer context#189
Open
halvaradop wants to merge 1 commit into
Open
refactor(core): improve user messages with clearer context#189halvaradop wants to merge 1 commit into
halvaradop wants to merge 1 commit into
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR refactors the entire error handling system by replacing seven custom error classes with a unified ChangesAura Error System Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 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
Comment on lines
70
to
+94
| const json = await response.json() | ||
| const token = OAuthAccessTokenResponse.safeParse(json) | ||
| if (!token.success) { | ||
| const { success, data } = OAuthAccessTokenErrorResponse.safeParse(json) | ||
| if (!success) { | ||
| logger?.log("INVALID_OAUTH_ACCESS_TOKEN_RESPONSE") | ||
| throw new OAuthProtocolError("invalid_request", "Invalid access token response format") | ||
| throw new AuraAuthError({ code: "INVALID_OAUTH_ACCESS_TOKEN_RES_FORMAT" }) | ||
| } | ||
| logger?.log("OAUTH_ACCESS_TOKEN_ERROR", { | ||
| structuredData: { | ||
| error: data.error, | ||
| error_description: data.error_description ?? "", | ||
| }, | ||
| }) | ||
| throw new OAuthProtocolError("INVALID_ACCESS_TOKEN", "Failed to retrieve access token") | ||
| throw new AuraAuthError({ code: "UNKNOWN_OAUTH_ACCESS_TOKEN_ERROR" }) | ||
| } | ||
|
|
||
| logger?.log("OAUTH_ACCESS_TOKEN_SUCCESS") | ||
| return token.data | ||
| } catch (error) { | ||
| if (error instanceof OAuthProtocolError) { | ||
| if (isAuraAuthError(error)) { | ||
| throw error | ||
| } | ||
| logger?.log("OAUTH_ACCESS_TOKEN_REQUEST_FAILED") | ||
| throw new OAuthProtocolError("server_error", "Failed to communicate with OAuth provider", "", { cause: error }) | ||
| throw new AuraAuthError({ code: "UNKNOWN_OAUTH_ACCESS_TOKEN_ERROR", cause: error }) |
There was a problem hiding this comment.
Malformed JSON is currently reported as an "unknown" OAuth failure in both callback fetchers.
Both implementations call response.json() inside the same broad try that also handles transport failures. If the provider returns a non-JSON or truncated body, the resulting parse error falls into the generic catch and gets reclassified as UNKNOWN_OAUTH_ACCESS_TOKEN_ERROR / UNKNOWN_OAUTH_USER_INFO_ERROR instead of the existing *_RES_FORMAT codes.
packages/core/src/actions/callback/access-token.ts#L70-L94: catch JSON parse failures separately and map them toINVALID_OAUTH_ACCESS_TOKEN_RES_FORMAT.packages/core/src/actions/callback/userinfo.ts#L64-L84: catch JSON parse failures separately and map them toINVALID_OAUTH_USER_INFO_RES_FORMAT.
🤖 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 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.
Comment on lines
23
to
27
| const baseURL = typeof authorizeConfig === "string" ? authorizeConfig : (authorizeConfig?.url ?? oauth.authorizeURL) | ||
| if (!baseURL) { | ||
| throw new AuthInternalError("INVALID_OAUTH_CONFIGURATION", "Missing authorization URL in OAuth provider configuration.") | ||
| throw new AuraAuthError({ code: "INVALID_OAUTH_PROVIDER_URL_CONFIG" }) | ||
| } | ||
| const url = new URL(baseURL) |
There was a problem hiding this comment.
Wrap new URL(baseURL) so malformed provider URLs still surface as AuraAuthError.
The truthiness check only catches missing values. A malformed configured URL still throws a native TypeError on new URL(baseURL), which bypasses the INVALID_OAUTH_PROVIDER_URL_CONFIG path and leaks an unstandardized error from the sign-in entrypoint.
Suggested fix
export const buildAuthorizationURL = (
oauth: OAuthProvider,
redirect_uri: string,
state: string,
code_challenge: string,
code_challenge_method: string
): string => {
const authorizeConfig = oauth.authorize
const baseURL = typeof authorizeConfig === "string" ? authorizeConfig : (authorizeConfig?.url ?? oauth.authorizeURL)
if (!baseURL) {
throw new AuraAuthError({ code: "INVALID_OAUTH_PROVIDER_URL_CONFIG" })
}
- const url = new URL(baseURL)
+ let url: URL
+ try {
+ url = new URL(baseURL)
+ } catch (cause) {
+ throw new AuraAuthError({ code: "INVALID_OAUTH_PROVIDER_URL_CONFIG", cause })
+ }
const authorizeParams = typeof authorizeConfig === "string" ? undefined : authorizeConfig?.params📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const baseURL = typeof authorizeConfig === "string" ? authorizeConfig : (authorizeConfig?.url ?? oauth.authorizeURL) | |
| if (!baseURL) { | |
| throw new AuthInternalError("INVALID_OAUTH_CONFIGURATION", "Missing authorization URL in OAuth provider configuration.") | |
| throw new AuraAuthError({ code: "INVALID_OAUTH_PROVIDER_URL_CONFIG" }) | |
| } | |
| const url = new URL(baseURL) | |
| const baseURL = typeof authorizeConfig === "string" ? authorizeConfig : (authorizeConfig?.url ?? oauth.authorizeURL) | |
| if (!baseURL) { | |
| throw new AuraAuthError({ code: "INVALID_OAUTH_PROVIDER_URL_CONFIG" }) | |
| } | |
| let url: URL | |
| try { | |
| url = new URL(baseURL) | |
| } catch (cause) { | |
| throw new AuraAuthError({ code: "INVALID_OAUTH_PROVIDER_URL_CONFIG", cause }) | |
| } |
🤖 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/signIn/authorization-url.ts` around lines 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.
| return createEndpointConfig({ | ||
| schemas: { | ||
| body: config?.schema ?? z.object({}), | ||
| body: config?.schema ?? z.object(), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Zod v4, is z.object() without a shape parameter supported, and what is the recommended empty-object schema fallback?
💡 Result:
In Zod v4, passing an empty object shape—specifically z.object({})—is fully supported and intentionally defined to represent an object that permits no properties [1]. When using z.object({}), the inferred type is Record<string, never>, which explicitly indicates an object with no keys [1][2]. Regarding an empty-object schema fallback: 1. Behavioral Change: In Zod v4, the behavior of .default() has been tightened [3]. Specifically, .default() now requires a value that strictly matches the output type of the schema [4][3]. Consequently, using .default({}) on an object schema is only valid if the schema itself is compatible with an empty object (i.e., it has no required fields, or all its fields have default values) [4][3][5]. 2. Recommended Alternatives: - Use .prefault(): If you intend to provide a default value that matches the input type and is parsed, use the .prefault() API, which was introduced in Zod v4 to provide a more flexible alternative to .default() [3][5]. - Ensure Compatibility: If you must use .default({}), ensure all fields within the object schema have defaults or are optional, as Zod v4 applies defaults more aggressively than previous versions [3][5][6]. As a technical note, while z.object({}) is valid, it is intentionally strict regarding the properties it accepts [2]. If you require an object that can accept any properties, you should use z.looseObject({}) (or equivalent loose configurations) instead, which allows for additional keys [1][7].
Citations:
- 1: https://github.com/colinhacks/zod/blob/c7805073/packages/zod/src/v4/classic/tests/object.test.ts
- 2:
strictObjectchanges behaviour in v4 colinhacks/zod#4823 - 3: How to use default values of optional object colinhacks/zod#5506
- 4: v4: cannot use empty object as default for schema with defaults - regression? colinhacks/zod#4308
- 5: [v4]
default()no longer impliesoptional()andoptional()must be the last chained method colinhacks/zod#4446 - 6: Breaking change in zod4 when extending a partial schema with defaults colinhacks/zod#5079
- 7: v4:
z.looseObject(obj).shapedoesn't seem to describe an object schema colinhacks/zod#4748
Use z.object({}) for the empty-body fallback
In Zod v4, the empty-object schema fallback should be an explicit empty shape (z.object({})); relying on z.object() with no shape is not the intended API for constructing an object schema.
Suggested fix
- body: config?.schema ?? z.object(),
+ body: config?.schema ?? z.object({}),🤖 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/signUp/signUp.ts` at 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.
Comment on lines
+77
to
80
| if (isAuraAuthError(error)) { | ||
| code = error.code | ||
| message = error.message | ||
| } |
There was a problem hiding this comment.
Use userMessage (not internal message) when projecting AuraAuthError into API payloads.
These catch paths now read error.message, but AuraAuthError separates internal diagnostics (message) from user-facing text (userMessage). This creates inconsistent client messaging and can expose internal wording.
packages/core/src/api/signIn.ts#L77-L80: setmessage = error.userMessagewhenisAuraAuthError(error)is true.packages/core/src/api/credentials.ts#L65-L68: setmessage = error.userMessagefor Aura errors before building the error object.packages/core/src/api/signOut.ts#L49-L52: setmessage = error.userMessagein the Aura error branch.packages/core/src/api/signUp.ts#L67-L70: setmessage = error.userMessagein the Aura error branch.packages/core/src/api/updateSession.ts#L66-L69: setmessage = error.userMessagein the Aura error branch.
🤖 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/signIn.ts` around lines 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).
Comment on lines
104
to
106
| if (!cookies) { | ||
| throw new AuthInternalError("COOKIE_NOT_FOUND", "No cookies found in response.") | ||
| throw new AuraAuthError({ code: "SET_COOKIE_NOT_FOUND" }) | ||
| } |
There was a problem hiding this comment.
Handle empty Set-Cookie collections as “not found.”
if (!cookies) does not catch an empty array, so a no-cookie response can be misreported as SET_COOKIE_INVALID_VALUE instead of SET_COOKIE_NOT_FOUND.
Proposed fix
- if (!cookies) {
+ if (!cookies || cookies.length === 0) {
throw new AuraAuthError({ code: "SET_COOKIE_NOT_FOUND" })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!cookies) { | |
| throw new AuthInternalError("COOKIE_NOT_FOUND", "No cookies found in response.") | |
| throw new AuraAuthError({ code: "SET_COOKIE_NOT_FOUND" }) | |
| } | |
| if (!cookies || cookies.length === 0) { | |
| throw new AuraAuthError({ code: "SET_COOKIE_NOT_FOUND" }) | |
| } |
🤖 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/cookie.ts` around lines 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`.
Comment on lines
+83
to
85
| } catch (error) { | ||
| throw new AuraAuthError({ code: "CSRF_TOKEN_MISSING", cause: error }) | ||
| } |
There was a problem hiding this comment.
Preserve mismatch errors in verifyCSRF catch path.
The catch currently rewrites every failure to CSRF_TOKEN_MISSING, which masks explicit CSRF_TOKEN_MISMATCH errors thrown above and breaks error-code semantics.
Proposed fix
- } catch (error) {
- throw new AuraAuthError({ code: "CSRF_TOKEN_MISSING", cause: error })
+ } catch (error) {
+ if (error instanceof AuraAuthError) throw error
+ throw new AuraAuthError({ code: "CSRF_TOKEN_MISMATCH", cause: error })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (error) { | |
| throw new AuraAuthError({ code: "CSRF_TOKEN_MISSING", cause: error }) | |
| } | |
| } catch (error) { | |
| if (error instanceof AuraAuthError) throw error | |
| throw new AuraAuthError({ code: "CSRF_TOKEN_MISMATCH", cause: error }) | |
| } |
🤖 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/crypto.ts` around lines 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.
Comment on lines
+407
to
+413
| CONFIG_BASE_URL_MISSING: { | ||
| type: "VALIDATION", | ||
| statusCode: 500, | ||
| name: "ConfigError", | ||
| message: "", | ||
| userMessage: "", | ||
| }, |
There was a problem hiding this comment.
Populate CONFIG_BASE_URL_MISSING with non-empty defaults.
This catalog entry is the only one here with empty message and userMessage. If it is thrown, AuraAuthError.message is blank and toResponse() emits { message: "" }, which breaks the response shape now surfaced directly by packages/core/src/router/errorHandler.ts and asserted in packages/core/test/instance.test.ts.
Possible fix
CONFIG_BASE_URL_MISSING: {
type: "VALIDATION",
statusCode: 500,
name: "ConfigError",
- message: "",
- userMessage: "",
+ message: "The application base URL could not be resolved from the current runtime configuration.",
+ userMessage: "The application base URL is missing. Set BASE_URL or provide valid host/proxy headers.",
},Also applies to: 729-733
🤖 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/unstable_error.ts` around lines 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).
Comment on lines
+61
to
62
| throw new AuraAuthError({ code: "SCHEMA_INVALID_MODE" }) | ||
| } |
There was a problem hiding this comment.
Use the correct error code for unsupported schema fallthroughs.
Line 61, Line 116, and Line 156 are schema-type fallbacks, but they throw SCHEMA_INVALID_MODE. That misclassifies the failure and can return the wrong catalog metadata to callers. These should use SCHEMA_UNSUPPORTED (or add an explicit runtime mode guard earlier if mode validation is the intended failure path).
Suggested fix
- throw new AuraAuthError({ code: "SCHEMA_INVALID_MODE" })
+ throw new AuraAuthError({ code: "SCHEMA_UNSUPPORTED" })- throw new AuraAuthError({ code: "SCHEMA_INVALID_MODE" })
+ throw new AuraAuthError({ code: "SCHEMA_UNSUPPORTED" })- throw new AuraAuthError({ code: "SCHEMA_INVALID_MODE" })
+ throw new AuraAuthError({ code: "SCHEMA_UNSUPPORTED" })Also applies to: 116-117, 156-157
🤖 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/validator/registry.ts` around lines 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".
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.
Summary by CodeRabbit
Release Notes
New Features
AuraAuthError) with specific error codes and user-friendly error messages.Improvements
Chores
^0.8.0.0.7.2.