feat(errors): typed ModelAccessDeniedError + sdk.checkCredentials() API - #991
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces comprehensive handling for "model access denied" errors across the LiteLLM provider integration. It adds a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant NeuroLink
participant Provider as LiteLLM/<br/>OpenAI Provider
participant ErrorClassifier as Error<br/>Classifier
Client->>NeuroLink: checkCredentials({provider, model?})
NeuroLink->>NeuroLink: Prepare probe request<br/>(disabled tools)
NeuroLink->>Provider: generate() call
alt Model Access Denied
Provider-->>NeuroLink: ModelAccessDeniedError
NeuroLink->>ErrorClassifier: Classify error
ErrorClassifier-->>NeuroLink: status: "denied"
else Authentication Error
Provider-->>NeuroLink: AuthenticationError
NeuroLink->>ErrorClassifier: Classify error
ErrorClassifier-->>NeuroLink: status: "expired" | "missing"
else Network/Connection Error
Provider-->>NeuroLink: Connection error
NeuroLink->>ErrorClassifier: Classify error
ErrorClassifier-->>NeuroLink: status: "network"
else Other/Unknown Error
Provider-->>NeuroLink: Generic error
NeuroLink->>ErrorClassifier: Classify error
ErrorClassifier-->>NeuroLink: status: "unknown"
else Success
Provider-->>NeuroLink: Generation succeeds
NeuroLink->>ErrorClassifier: No error
ErrorClassifier-->>NeuroLink: status: "ok"
end
NeuroLink-->>Client: {provider, status, detail}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Documentation Validation Results🚀 Documentation validation passed!
📦 Build artifact uploaded successfully. Ready for deployment preview. Commit: |
There was a problem hiding this comment.
Pull request overview
This PR improves SDK error typing and credential diagnostics to address Curator Issue P1-1 (LiteLLM 403 “team not allowed to access model” surfaced as an untyped error, and no sdk.checkCredentials() API).
Changes:
- Add
ModelAccessDeniedErrorplus helpers to detect/parse LiteLLM “allowed models” from error messages. - Update LiteLLM + OpenAI provider error formatting and mark
ModelAccessDeniedErroras non-retryable. - Add
NeuroLink.checkCredentials()and a continuous test script + helper utilities + documentation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/helpers/envGuard.ts | Adds env var skip helper and provider-error detection helper for test scripts. |
| test/continuous-test-suite-issue-01-model-access.ts | Adds a standalone continuous test for typed errors + checkCredentials() surface. |
| src/lib/types/errors.ts | Introduces ModelAccessDeniedError and message parsing/detection helpers. |
| src/lib/providers/openAI.ts | Extends OpenAI auth error detection/message handling. |
| src/lib/providers/litellm.ts | Detects LiteLLM team-denied model access and surfaces ModelAccessDeniedError. |
| src/lib/neurolink.ts | Adds non-retryable short-circuit for model access denied and implements checkCredentials(). |
| docs/curator-feedback-fixes/issue-01-model-access-denied.md | Documents the issue, approach, and verification steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| message.includes("API_KEY_INVALID") || | ||
| message.includes("Invalid API key") || | ||
| errorType === "invalid_api_key" | ||
| message.includes("Incorrect API key") || | ||
| errorType === "invalid_api_key" || | ||
| errorType === "invalid_request_error" | ||
| ) { |
There was a problem hiding this comment.
errorType === "invalid_request_error" is not a reliable signal of invalid credentials for OpenAI. OpenAI uses invalid_request_error for many non-auth failures (bad params, context length, etc.), so this will misclassify legitimate request/validation errors as AuthenticationError and hide the real cause. Prefer checking errorType === "invalid_api_key" / authentication_error, or (if available on the thrown error) an HTTP 401 status code and/or the specific "Incorrect API key" message substring.
| message.includes("Incorrect API key") || | ||
| message.includes("Invalid API key") | ||
| ? message | ||
| : "Invalid OpenAI API key. Please check your OPENAI_API_KEY environment variable.", |
There was a problem hiding this comment.
Returning the raw provider message for "Incorrect API key" / "Invalid API key" can leak the supplied key in the exception text (OpenAI commonly includes the provided key after a colon). Consider keeping the generic message or sanitizing the provider message (e.g., strip any ...provided: <key> segment) before returning it.
| message.includes("Incorrect API key") || | |
| message.includes("Invalid API key") | |
| ? message | |
| : "Invalid OpenAI API key. Please check your OPENAI_API_KEY environment variable.", | |
| "Invalid OpenAI API key. Please check your OPENAI_API_KEY environment variable.", |
| * Curator P1-1: synchronous credential health check for a single provider. | ||
| * | ||
| * Drives a tiny real call against the provider (1-token completion or | ||
| * `/models` listing depending on provider) to confirm the configured | ||
| * credentials are valid. Useful at startup so a service can refuse to | ||
| * boot if its primary provider's credentials are broken instead of | ||
| * discovering the problem on first user request. |
There was a problem hiding this comment.
The JSDoc says this is a "synchronous" health check and may call a /models listing, but the implementation is async and always calls this.generate(...). Please align the documentation with the actual behavior (or implement the provider-specific /models probe if that’s intended).
| * Curator P1-1: synchronous credential health check for a single provider. | |
| * | |
| * Drives a tiny real call against the provider (1-token completion or | |
| * `/models` listing depending on provider) to confirm the configured | |
| * credentials are valid. Useful at startup so a service can refuse to | |
| * boot if its primary provider's credentials are broken instead of | |
| * discovering the problem on first user request. | |
| * Curator P1-1: asynchronous credential health check for a single provider. | |
| * | |
| * Drives a tiny real generation call against the provider to confirm the | |
| * configured credentials are valid. Useful at startup so a service can | |
| * refuse to boot if its primary provider's credentials are broken instead | |
| * of discovering the problem on first user request. |
| provider: provider as never, | ||
| ...(model && { model }), | ||
| input: { text: probeText }, | ||
| maxTokens: 16, |
There was a problem hiding this comment.
The comment and code mention a "1-token probe", but the call uses maxTokens: 16. If the intent is a minimal-cost probe, consider setting maxTokens to 1 (and/or updating the docs to match).
| maxTokens: 16, | |
| maxTokens: 1, |
| lower.includes("authentication") || | ||
| lower.includes("401") || | ||
| lower.includes("invalid api key") || | ||
| lower.includes("incorrect api key") || | ||
| lower.includes("api_key_invalid") || | ||
| lower.includes("token has expired") || |
There was a problem hiding this comment.
checkCredentials() documents "expired" as covering 401/403, but the classification only looks for "401" (and some auth phrases). This means a 403 auth/permission failure could fall through to unknown. Consider also checking for "403" and/or using typed errors (AuthenticationError / AuthorizationError) when available.
| lower.includes("authentication") || | |
| lower.includes("401") || | |
| lower.includes("invalid api key") || | |
| lower.includes("incorrect api key") || | |
| lower.includes("api_key_invalid") || | |
| lower.includes("token has expired") || | |
| lower.includes("authentication") || | |
| lower.includes("authorization") || | |
| lower.includes("unauthorized") || | |
| lower.includes("forbidden") || | |
| lower.includes("permission denied") || | |
| lower.includes("401") || | |
| lower.includes("403") || | |
| lower.includes("invalid api key") || | |
| lower.includes("incorrect api key") || | |
| lower.includes("api_key_invalid") || | |
| lower.includes("token has expired") || | |
| lower.includes("access token expired") || |
| console.log( | ||
| `\n${colors.bright}Results:${colors.reset} ${passed} passed, ${failed} failed, ${skipped} skipped`, | ||
| ); | ||
| process.exit(0); // bug repro: failed > 0 expected |
There was a problem hiding this comment.
This test runner always exits 0 even when there are FAIL outcomes, which makes it hard to use in automation and is inconsistent with other continuous test suites that exit non-zero on failure. Consider process.exit(failed === 0 ? 0 : 1) (while still printing the summary).
| process.exit(0); // bug repro: failed > 0 expected | |
| process.exit(failed === 0 ? 0 : 1); |
| * Detect provider/credential errors that mean "could not run the test" | ||
| * rather than "the test reproduced the bug". Mirrors the convention used | ||
| * by continuous-test-suite-credentials.ts. | ||
| */ | ||
| export function isExpectedProviderError(msg: string): boolean { | ||
| const lower = msg.toLowerCase(); | ||
| return [ | ||
| "api key", | ||
| "api_key", | ||
| "authentication", | ||
| "rate limit", | ||
| "quota", |
There was a problem hiding this comment.
The comment says this helper "Mirrors the convention used by continuous-test-suite-credentials.ts", but the keyword list differs (e.g., credentials suite also checks for "permission denied", "403", "failed to", and "not found"). Either update the list to match the referenced convention, or adjust the comment so it doesn’t imply exact parity.
|
@coderabbitai review Pushed 965b609d addressing CodeQL alert #243 ( |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
965b609 to
9f768b0
Compare
|
Force-pushed squash to a single commit @coderabbitai review the squashed commit. |
|
🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/lib/types/errors.ts (1)
261-291: Minor: advance past the matchedmodelstoken on misses to avoid redundant rescans.When
=or[doesn't follow, the walker restarts the search fromidx + 1, which re-examines the bytes inside the currentmodelstoken. Functionally fine, but advancing past the token is both faster and clearer about intent. Same reasoning when]is missing — currently we abort the whole search, but a malformedmodels=[…earlier in the message will mask a well-formed list later.♻️ Proposed refactor
const lower = message.toLowerCase(); let idx = lower.indexOf("models", 0); while (idx !== -1) { let cursor = idx + "models".length; + const nextSearchFrom = cursor; // Skip whitespace while (cursor < message.length && /\s/.test(message[cursor])) { cursor++; } if (message[cursor] !== "=") { - idx = lower.indexOf("models", idx + 1); + idx = lower.indexOf("models", nextSearchFrom); continue; } cursor++; while (cursor < message.length && /\s/.test(message[cursor])) { cursor++; } if (message[cursor] !== "[") { - idx = lower.indexOf("models", idx + 1); + idx = lower.indexOf("models", nextSearchFrom); continue; } const open = cursor; const close = message.indexOf("]", open + 1); if (close === -1) { - return undefined; + idx = lower.indexOf("models", nextSearchFrom); + continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/errors.ts` around lines 261 - 291, The loop that scans for the "models" token currently resets idx to idx + 1 on mismatches and returns undefined if a closing ] is missing; update the logic in the scanning block that uses idx, lower, and message so that on mismatches you advance past the entire "models" token (e.g., idx = lower.indexOf("models", idx + "models".length)) instead of idx + 1 to avoid re-scanning bytes inside the token, and when close === -1 do not return undefined immediately but continue searching after the current "models" occurrence (advance idx past the token) so malformed early fragments don't hide a well-formed list later.test/continuous-test-suite-issue-01-model-access.ts (1)
93-93: String-based constructor name check is brittle.Comparing
ctorName === "ModelAccessDeniedError"/"AuthenticationError"works today but breaks under name-mangling minifiers, class-extending wrappers, or when a future refactor renames the class without updating tests. Since both classes are exported from the SDK, aninstanceofcheck is more robust and self-documenting. Optional given this is a smoke harness, but worth considering.♻️ Sketch
-import { NeuroLink } from "../dist/index.js"; +import { NeuroLink, ModelAccessDeniedError, AuthenticationError } from "../dist/index.js"; ... - const isTypedAccessError = ctorName === "ModelAccessDeniedError"; + const isTypedAccessError = captured instanceof ModelAccessDeniedError; ... - if (ctorName === "AuthenticationError") { + if (captured instanceof AuthenticationError) {Also applies to: 204-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-issue-01-model-access.ts` at line 93, Replace the brittle string-based constructor name checks that compare ctorName === "ModelAccessDeniedError" (and similarly for "AuthenticationError") with robust instanceof checks using the actual exported error classes; import or reference the SDK's ModelAccessDeniedError and AuthenticationError and change the logic that computes isTypedAccessError (and the similar check at the other location) to use e.g. err instanceof ModelAccessDeniedError / err instanceof AuthenticationError instead of comparing ctorName, so the test survives minification/renames and subclassing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/curator-feedback-fixes/issue-01-model-access-denied.md`:
- Line 77: The doc's `"expired"` semantics must match the implementation: update
the documentation text to state that `"expired"` covers credentials rejected due
to authentication/authorization errors (e.g., 401 and 403 responses and auth
messages like "invalid api key" or "token has expired") to align with the logic
in checkCredentials and the handling in src/lib/neurolink.ts; alternatively, if
you prefer tightening behavior instead, modify checkCredentials and its callers
to only treat 401 as `"expired"` and adjust any branches that currently classify
403 or auth-message matches as `"expired"`.
In `@src/lib/neurolink.ts`:
- Around line 8055-8097: The checkCredentials function currently always calls
this.generate (symbol: checkCredentials and generate) which verifies a default
model rather than provider-level auth; change the logic so when input.model is
omitted you perform a provider-level probe (e.g., call the provider-specific
models listing or health endpoint such as a listModels/listProviders or /models
call on the provider client) to confirm credentials/auth at the provider scope,
and only fall back to a model probe (this.generate) when a model is explicitly
provided; map errors from the provider-level call to the same status values
("ok", "missing", "expired", "denied", "network", "unknown") and include clear
detail strings so checkCredentials correctly reports provider usability
independent of any chosen default model.
- Around line 325-330: The ModelAccessDeniedError check in the terminal-error
helper currently returns true unconditionally, which aborts the provider
fallback chain (affecting directProviderGeneration()); change the logic so
ModelAccessDeniedError only signals terminality when it is genuinely
non-retryable for the whole request — e.g. when the request explicitly pinned
the provider/model or when the helper is invoked in a retry-suppressed context.
Update the helper signature or use the existing context flags (e.g., a
suppressRetries/isPinned boolean) and replace the unconditional "if (error
instanceof ModelAccessDeniedError) return true;" with a conditional that returns
true only when providerPinned === true or suppressRetries === true, otherwise
return false so fallback to other providers may proceed.
In `@src/lib/providers/openAI.ts`:
- Around line 328-342: The current logic incorrectly treats any error with
errorType === "invalid_request_error" as an AuthenticationError and also
contains an unreachable check for errorType === "invalid_api_key"; update the
conditional in the OpenAI provider (the block that returns new
AuthenticationError using this.providerName and message) to: remove the
unreachable errorType === "invalid_api_key" check entirely, and only map
invalid_request_error to AuthenticationError when the actual error message
contains auth-specific substrings ("Invalid API key" or "Incorrect API key");
keep the existing behavior of returning the original message when those
substrings match and otherwise do not classify invalid_request_error as
AuthenticationError (so other invalid_request_error cases fall through to
non-auth error handling used elsewhere, e.g., NeuroLink.checkCredentials).
In `@test/continuous-test-suite-issue-01-model-access.ts`:
- Around line 225-232: The test currently unconditionally calls process.exit(0),
masking test failures; change this to exit non‑zero when failures exist by
replacing process.exit(0) with a conditional exit like process.exit(failed > 0 ?
1 : 0) so the CI fails when the computed failed count (from results) is > 0;
keep the existing console output and use the variables failed and results
referenced in the snippet.
- Around line 197-199: The test currently treats a successful sdk.generate()
with the deliberately-invalid key as a SKIP; change this to fail the test
instead. In the block that checks if (!captured) replace the call to
record(testName, "SKIP", "expected rejection — got success") with
record(testName, "FAIL", "expected rejection — got success (invalid key
accepted)") so the behavior matches test_1_1_raw_error_surface and surfaces a
regression when an invalid key succeeds; keep the testName and captured
variables and the sdk.generate() call unchanged.
---
Nitpick comments:
In `@src/lib/types/errors.ts`:
- Around line 261-291: The loop that scans for the "models" token currently
resets idx to idx + 1 on mismatches and returns undefined if a closing ] is
missing; update the logic in the scanning block that uses idx, lower, and
message so that on mismatches you advance past the entire "models" token (e.g.,
idx = lower.indexOf("models", idx + "models".length)) instead of idx + 1 to
avoid re-scanning bytes inside the token, and when close === -1 do not return
undefined immediately but continue searching after the current "models"
occurrence (advance idx past the token) so malformed early fragments don't hide
a well-formed list later.
In `@test/continuous-test-suite-issue-01-model-access.ts`:
- Line 93: Replace the brittle string-based constructor name checks that compare
ctorName === "ModelAccessDeniedError" (and similarly for "AuthenticationError")
with robust instanceof checks using the actual exported error classes; import or
reference the SDK's ModelAccessDeniedError and AuthenticationError and change
the logic that computes isTypedAccessError (and the similar check at the other
location) to use e.g. err instanceof ModelAccessDeniedError / err instanceof
AuthenticationError instead of comparing ctorName, so the test survives
minification/renames and subclassing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f7f9bf9-043e-4551-916c-f49d90e9c26d
📒 Files selected for processing (7)
docs/curator-feedback-fixes/issue-01-model-access-denied.mdsrc/lib/neurolink.tssrc/lib/providers/litellm.tssrc/lib/providers/openAI.tssrc/lib/types/errors.tstest/continuous-test-suite-issue-01-model-access.tstest/helpers/envGuard.ts
| // Curator P1-1: model-access-denied is permanent for the (provider, model) | ||
| // pair until the team whitelist changes. Retrying with the same config | ||
| // would just waste a second roundtrip. Caller / fallback-orchestrator | ||
| // should pick a different model. | ||
| if (error instanceof ModelAccessDeniedError) { | ||
| return true; |
There was a problem hiding this comment.
Don't short-circuit cross-provider fallback on ModelAccessDeniedError.
This helper is also used by directProviderGeneration(), so returning true here now aborts the entire provider fallback chain after the first denied provider/model pair. In auto/fallback mode that turns a provider-specific denial into a hard failure, even when the next provider could still satisfy the request. Please scope this to retry suppression only, or only treat it as terminal when the provider was explicitly pinned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/neurolink.ts` around lines 325 - 330, The ModelAccessDeniedError
check in the terminal-error helper currently returns true unconditionally, which
aborts the provider fallback chain (affecting directProviderGeneration());
change the logic so ModelAccessDeniedError only signals terminality when it is
genuinely non-retryable for the whole request — e.g. when the request explicitly
pinned the provider/model or when the helper is invoked in a retry-suppressed
context. Update the helper signature or use the existing context flags (e.g., a
suppressRetries/isPinned boolean) and replace the unconditional "if (error
instanceof ModelAccessDeniedError) return true;" with a conditional that returns
true only when providerPinned === true or suppressRetries === true, otherwise
return false so fallback to other providers may proceed.
| if ( | ||
| message.includes("API_KEY_INVALID") || | ||
| message.includes("Invalid API key") || | ||
| errorType === "invalid_api_key" | ||
| message.includes("Incorrect API key") || | ||
| errorType === "invalid_api_key" || | ||
| errorType === "invalid_request_error" | ||
| ) { | ||
| return new AuthenticationError( | ||
| "Invalid OpenAI API key. Please check your OPENAI_API_KEY environment variable.", | ||
| message.includes("Incorrect API key") || | ||
| message.includes("Invalid API key") | ||
| ? message | ||
| : "Invalid OpenAI API key. Please check your OPENAI_API_KEY environment variable.", | ||
| this.providerName, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAI API error types: what does invalid_request_error cover vs invalid_api_key vs authentication_error?
💡 Result:
OpenAI API distinguishes these error types as follows: - invalid_request_error: A general error type (HTTP 400 or 401) for malformed requests or invalid parameters, including cases like an incorrect API key. The error object includes a 'code': 'invalid_api_key' for specificity, e.g., {'error': {'type': 'invalid_request_error', 'code': 'invalid_api_key', 'message': 'Incorrect API key provided...'}}. It covers bad requests beyond just auth. - invalid_api_key: Not a standalone error type but a specific 'code' within invalid_request_error, signaling the API key is wrong (typo, revoked, wrong org, etc.). - authentication_error: A Python SDK exception (HTTP 401) for invalid, expired, or revoked API keys/tokens. Maps to raw API 401 errors like 'Invalid Authentication' or 'Incorrect API key provided', but distinct from SDK's BadRequestError (formerly InvalidRequestError) for non-auth issues.
Citations:
- 1: https://developers.openai.com/api/docs/guides/error-codes
- 2: https://platform.openai.com/docs/guides/error-codes/api-errors
- 3: https://developers.openai.com/api/docs/guides/error-codes/
- 4: Error when using OPEN AI API assafelovic/gpt-researcher#278
- 5: https://community.openai.com/t/invalid-api-key/29936
- 6: https://community.openai.com/t/openai-chat-list-of-error-codes-and-types/357791
- 7: https://community.openai.com/t/authenticationerror-error-code-401/766947
🏁 Script executed:
cat -n src/lib/providers/openAI.ts | head -360 | tail -80Repository: juspay/neurolink
Length of output: 3067
🏁 Script executed:
rg -A 5 "import.*openai|from.*openai" src/lib/providers/openAI.tsRepository: juspay/neurolink
Length of output: 241
🏁 Script executed:
cat package.json | jq '.dependencies, .devDependencies' | grep -i openaiRepository: juspay/neurolink
Length of output: 91
🏁 Script executed:
rg -B 5 "formatProviderError" src/lib/providers/openAI.ts | head -40Repository: juspay/neurolink
Length of output: 143
🏁 Script executed:
web_search
`@ai-sdk/openai` error object structure type field invalid_api_key invalid_request_errorRepository: juspay/neurolink
Length of output: 172
🏁 Script executed:
rg -B 10 -A 2 "\.formatProviderError\(" src/lib/providers/openAI.tsRepository: juspay/neurolink
Length of output: 42
🏁 Script executed:
rg "catch.*error" src/lib/providers/openAI.ts -A 3Repository: juspay/neurolink
Length of output: 901
🌐 Web query:
@ai-sdk/openai error object structure type field invalid_api_key invalid_request_error
💡 Result:
In the @ai-sdk/openai package (part of Vercel AI SDK), API errors from OpenAI, such as invalid_api_key within invalid_request_error, are wrapped in an AI_APICallError object. The structure of the AI_APICallError includes: - url: string - The API endpoint URL that failed. - requestBodyValues: object - The request body sent. - statusCode: number (typically 401 for invalid_api_key) - HTTP status code. - responseHeaders: object - Response headers. - responseBody: object|string - The raw OpenAI error response, e.g. { error: { message: '...', type: 'invalid_request_error', code: 'invalid_api_key', param: null } }. - isRetryable: boolean - false for auth errors like 401. - data: object - Additional error data. To check: import { APICallError } from 'ai'; if (APICallError.isInstance(error)) { ... } Access OpenAI details via error.responseBody.error.code === 'invalid_api_key' and error.responseBody.error.type === 'invalid_request_error'. Handle in try/catch or stream 'error' parts. Official docs confirm this uniform error wrapping across providers.
Citations:
- 1: https://ai-sdk.dev/v4/docs/reference/ai-sdk-errors/ai-api-call-error
- 2: https://sdk.vercel.ai/docs/reference/ai-sdk-errors/ai-api-call-error
- 3: AI Gateway breaks SDK retry: GatewayInternalServerError not recognized by maxRetries vercel/ai#14216
- 4: Bad response from OpenAI throws uncatchable error vercel/ai#1722
- 5:
@ai-sdk/openaidoes not handlequotarelated errors correctly vercel/ai#10304 - 6: https://community.openai.com/t/invalid-api-key/29936
- 7: https://community.openai.com/t/invalid-request-error-in-chatgpt-api/81213
- 8: https://developers.openai.com/api/docs/guides/error-codes
- 9: https://developers.openai.com/api/docs/guides/error-codes/
🏁 Script executed:
rg -B 5 -A 15 "handleProviderError" src/lib/providers/openAI.tsRepository: juspay/neurolink
Length of output: 1733
🏁 Script executed:
rg -B 5 -A 15 "private handleProviderError|handleProviderError.*=" src/lib/providers/openAI.tsRepository: juspay/neurolink
Length of output: 42
🏁 Script executed:
rg -B 2 -A 10 "handleProviderError\s*\(" src/lib/providers/ --include="*.ts"Repository: juspay/neurolink
Length of output: 502
🏁 Script executed:
rg "handleProviderError" src/lib/providers/ --include="*.ts" -lRepository: juspay/neurolink
Length of output: 502
🏁 Script executed:
rg "handleProviderError" src/lib/providers/ -A 10Repository: juspay/neurolink
Length of output: 21350
🏁 Script executed:
grep -n "handleProviderError" src/lib/providers/BaseProvider.tsRepository: juspay/neurolink
Length of output: 127
🏁 Script executed:
find src/lib/providers -name "*Base*" -o -name "*base*"Repository: juspay/neurolink
Length of output: 103
🏁 Script executed:
rg "class BaseProvider" src/lib/providersRepository: juspay/neurolink
Length of output: 42
🏁 Script executed:
head -50 src/lib/providers/openAI.ts | grep -E "import|export|class"Repository: juspay/neurolink
Length of output: 931
🏁 Script executed:
rg "handleProviderError.*=" src/lib/providers/ -A 5Repository: juspay/neurolink
Length of output: 42
🏁 Script executed:
cat -n src/lib/core/baseProvider.ts | head -100Repository: juspay/neurolink
Length of output: 4490
🏁 Script executed:
rg "handleProviderError" src/lib/core/baseProvider.ts -B 5 -A 10Repository: juspay/neurolink
Length of output: 2651
🏁 Script executed:
cat -n src/lib/core/baseProvider.ts | grep -A 30 "protected handleProviderError"Repository: juspay/neurolink
Length of output: 1554
🏁 Script executed:
rg "protected abstract formatProviderError|public formatProviderError" src/lib/core/baseProvider.ts -A 5Repository: juspay/neurolink
Length of output: 317
🏁 Script executed:
rg "AI_APICallError|APICallError" src/lib/providers/Repository: juspay/neurolink
Length of output: 42
🏁 Script executed:
rg "responseBody.*error" src/lib/providers/openAI.ts -B 2 -A 2Repository: juspay/neurolink
Length of output: 42
🏁 Script executed:
cat -n src/lib/providers/openAI.ts | sed -n '313,360p'Repository: juspay/neurolink
Length of output: 1901
🏁 Script executed:
rg "errorObj|error as" src/lib/providers/openAI.ts -B 2 -A 3 | head -60Repository: juspay/neurolink
Length of output: 385
Major: invalid_request_error is too broad to map to AuthenticationError, and invalid_api_key check is unreachable due to type/code field mismatch.
invalid_request_error is OpenAI's generic 400 error type for malformed requests, invalid parameters, schema issues, and context-length overflows—not just bad credentials. Mapping it to AuthenticationError causes false-positive auth failures for unrelated request-shape problems. Additionally, line 332's check errorType === "invalid_api_key" is broken: the @ai-sdk/openai library nests OpenAI errors at responseBody.error, where invalid_api_key is a code field, not a type field. This check will never match because you're comparing the type field against a code value.
Consequences:
NeuroLink.checkCredentials()returnsstatus: "expired"for transient request errors, creating false positives.- Lines 336–337 only echo the original message for
"Incorrect API key"/"Invalid API key"substrings, soinvalid_request_errortriggers always falls back to canned text, losing the real diagnostic. - Retry/fallback logic treats
AuthenticationErroras terminal, short-circuiting on errors callers could fix.
The correct narrow signal is errorType === "invalid_request_error" only when combined with auth-specific message checks. Drop the invalid_api_key type check entirely (it's unreachable). The message-based checks ("Invalid API key", "Incorrect API key") already cover the actual auth-specific cases.
🛡️ Proposed fix
if (
message.includes("API_KEY_INVALID") ||
message.includes("Invalid API key") ||
message.includes("Incorrect API key") ||
- errorType === "invalid_api_key" ||
- errorType === "invalid_request_error"
+ errorType === "invalid_api_key"
) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/providers/openAI.ts` around lines 328 - 342, The current logic
incorrectly treats any error with errorType === "invalid_request_error" as an
AuthenticationError and also contains an unreachable check for errorType ===
"invalid_api_key"; update the conditional in the OpenAI provider (the block that
returns new AuthenticationError using this.providerName and message) to: remove
the unreachable errorType === "invalid_api_key" check entirely, and only map
invalid_request_error to AuthenticationError when the actual error message
contains auth-specific substrings ("Invalid API key" or "Incorrect API key");
keep the existing behavior of returning the original message when those
substrings match and otherwise do not classify invalid_request_error as
AuthenticationError (so other invalid_request_error cases fall through to
non-auth error handling used elsewhere, e.g., NeuroLink.checkCredentials).
| const passed = results.filter((r) => r.outcome === "PASS").length; | ||
| const failed = results.filter((r) => r.outcome === "FAIL").length; | ||
| const skipped = results.filter((r) => r.outcome === "SKIP").length; | ||
| console.log( | ||
| `\n${colors.bright}Results:${colors.reset} ${passed} passed, ${failed} failed, ${skipped} skipped`, | ||
| ); | ||
| process.exit(0); // bug repro: failed > 0 expected | ||
| } |
There was a problem hiding this comment.
process.exit(0) masks failures from CI.
The exit code is hard-coded to 0 regardless of failed. The trailing comment (// bug repro: failed > 0 expected) made sense when the suite was reproducing the bug pre-fix, but with the fix landing in this PR the suite is now an assertion of correct behavior. Leaving the unconditional exit(0) means a regression (e.g. a future change that breaks ModelAccessDeniedError typing or checkCredentials exposure) will silently pass CI.
🔧 Proposed fix
console.log(
`\n${colors.bright}Results:${colors.reset} ${passed} passed, ${failed} failed, ${skipped} skipped`,
);
- process.exit(0); // bug repro: failed > 0 expected
+ process.exit(failed > 0 ? 1 : 0);
}📝 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.
| const passed = results.filter((r) => r.outcome === "PASS").length; | |
| const failed = results.filter((r) => r.outcome === "FAIL").length; | |
| const skipped = results.filter((r) => r.outcome === "SKIP").length; | |
| console.log( | |
| `\n${colors.bright}Results:${colors.reset} ${passed} passed, ${failed} failed, ${skipped} skipped`, | |
| ); | |
| process.exit(0); // bug repro: failed > 0 expected | |
| } | |
| const passed = results.filter((r) => r.outcome === "PASS").length; | |
| const failed = results.filter((r) => r.outcome === "FAIL").length; | |
| const skipped = results.filter((r) => r.outcome === "SKIP").length; | |
| console.log( | |
| `\n${colors.bright}Results:${colors.reset} ${passed} passed, ${failed} failed, ${skipped} skipped`, | |
| ); | |
| process.exit(failed > 0 ? 1 : 0); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/continuous-test-suite-issue-01-model-access.ts` around lines 225 - 232,
The test currently unconditionally calls process.exit(0), masking test failures;
change this to exit non‑zero when failures exist by replacing process.exit(0)
with a conditional exit like process.exit(failed > 0 ? 1 : 0) so the CI fails
when the computed failed count (from results) is > 0; keep the existing console
output and use the variables failed and results referenced in the snippet.
|
Force-pushed addressing reviewer Findings #4 + #5 plus applying same recipe as #988/#989/#990:
Suite re-verified: 4/4 PASS against real LiteLLM (typed ModelAccessDeniedError, 19 allowedModels parsed, checkCredentials present, OpenAI bad-key surfaces typed AuthenticationError). @coderabbitai full review |
9f768b0 to
328c906
Compare
|
Kicking off a full review of the updated branch now. 🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
Force-pushed addressing reviewer Finding #5 follow-up: The previous round's skip-path fix on test 1.2 was lost during a later rebase/amend cycle — committed file DNS / network failures now report as SKIP (provider unavailable) instead of false-failing as bug-confirmed. Suite re-verified: 4/4 PASS. @coderabbitai full review |
328c906 to
420f63f
Compare
|
🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
Curator P1-1: when LiteLLM returned 403 with "team not allowed to access
model. This team can only access models=['glm-latest', 'kimi-latest',
'open-large']", the SDK surfaced a raw error without a typed class and
without parsing the allowed_models list. There was no
sdk.checkCredentials() API for synchronous health-check at startup.
Three deliverables:
(1) New typed error class `ModelAccessDeniedError extends ProviderError`
in src/lib/types/errors.ts, with `requestedModel`, `allowedModels`, and
code "MODEL_ACCESS_DENIED". Plus `parseAllowedModels(message)` and
`isModelAccessDeniedMessage(message)` helpers.
`parseAllowedModels` uses an indexOf/slice walker (no regex
backtracking, capped at 10 KB input) so a crafted long provider error
body can't trigger ReDoS. CodeQL's js/polynomial-redos rule passes.
(2) LiteLLM provider error formatter (src/lib/providers/litellm.ts)
detects the team-denied pattern before the generic auth check and
surfaces ModelAccessDeniedError with allowedModels populated. Added to
the non-retryable short-circuit list in neurolink.ts since the rejection
is permanent for that (provider, model) pair.
OpenAI formatter extended to match real 401 messages ("Incorrect API
key", error type "invalid_request_error") so wrong-key responses
surface as typed AuthenticationError instead of plain Error.
(3) New `sdk.checkCredentials({ provider, model? })` method on NeuroLink
that probes with a 1-token call and returns
{ provider, status, detail } where status is one of "ok", "missing",
"expired", "denied", "network", "unknown". Lets services refuse to boot
when their primary provider's credentials are broken instead of
discovering the problem on first user request.
Reproduction (real LiteLLM at http://grid.ai.juspay.net/v1):
before: 0/4 passing
after: 4/4 passing — typed ModelAccessDeniedError with 19 allowedModels
parsed from real proxy; checkCredentials present; bad OpenAI key
surfaces typed AuthenticationError
Backward-compatible: ModelAccessDeniedError extends ProviderError so any
caller catching the parent class continues to work.
420f63f to
d6331dd
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
🎉 This PR is included in version 9.59.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Curator P1-1: when LiteLLM returned 403 with
team not allowed to access model. This team can only access models=['glm-latest', 'kimi-latest', 'open-large'], the SDK surfaced a raw error without a typed class and without parsing theallowed_modelslist. There was nosdk.checkCredentials()API.Three deliverables
ModelAccessDeniedError— new typed error class extendingProviderError, withrequestedModel,allowedModels,code: "MODEL_ACCESS_DENIED". PlusparseAllowedModels()andisModelAccessDeniedMessage()helpers.ModelAccessDeniedErrorwithallowedModelsparsed from the body; OpenAI formatter extended to surface typedAuthenticationErroron real 401 ("Incorrect API key").ModelAccessDeniedErroradded to the non-retryable short-circuit list inneurolink.ts.sdk.checkCredentials({ provider, model? })— 1-token probe call, returns{ provider, status, detail }wherestatusis"ok" | "missing" | "expired" | "denied" | "network" | "unknown". Services can refuse to boot when their primary provider's credentials are broken.Reproduction (real LiteLLM at LITELLM_BASE_URL)
Backward compatibility
ModelAccessDeniedError extends ProviderError— callers catching plainError/ProviderErrorcontinue to work.ProviderError; now as the typed subclass. Same parent, same message, narrower type.parseAllowedModels/isModelAccessDeniedMessageare new exports.checkCredentials()is additive onNeuroLink.Verification
Test plan
allowedModelsarray (19 entries) parsed from real proxy responseModelAccessDeniedErrorshort-circuits the retry chain (no wasted retries)AuthenticationErrorcheckCredentials({ provider: "litellm" })returnsstatus: "ok"against working setupSummary by CodeRabbit
New Features
checkCredentials()method to diagnose provider authentication and model access status with structured reporting.Improvements