-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Catch errors when SECRETS is empty or doesnt have the right key #51
Conversation
WalkthroughWalkthroughThe recent updates enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Secrets
User->>App: Call initializeOpenAI()
App->>Secrets: Read API key
alt Successful retrieval
Secrets-->>App: Return API key
App->>User: Initialization successful
else Retrieval failed
Secrets-->>App: Throw error
App->>App: Log error
App->>User: Display error message
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
@coderabbitai full review |
Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/init.test.ts (2 hunks)
- src/init.ts (1 hunks)
Additional context used
Path-based instructions (2)
src/init.test.ts (1)
Pattern
**/*.ts
: Review these typescript files for best practices, readability, and DRY.src/init.ts (1)
Pattern
**/*.ts
: Review these typescript files for best practices, readability, and DRY.
Additional comments not posted (7)
src/init.test.ts (6)
95-117
: LGTM!The test case for setting image models correctly is well-structured and covers the necessary assertions.
119-141
: LGTM!The test case for setting embedding models correctly is well-structured and covers the necessary assertions.
143-160
: LGTM!The test case for handling missing settings gracefully is well-structured and covers the necessary assertions.
162-175
: LGTM!The test case for handling an empty API key is well-structured and covers the necessary assertions.
177-190
: LGTM!The test case for handling a missing API secret is well-structured and covers the necessary assertions.
192-207
: LGTM!The test case for handling a non-existent secrets page is well-structured and covers the necessary assertions.
src/init.ts (1)
217-227
: LGTM!The changes to the
configureSelectedModel
function improve error handling by adding atry-catch
block around the API key retrieval logic. This ensures that errors are caught and logged, and a user-friendly error message is provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai generate additional tests in this file for src/init.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@justyns, I've generated additional tests for the initializeOpenAI
and configureSelectedModel
functions based on their implementations. Here are the new test cases:
import { assertEquals, assertThrowsAsync } from "https://deno.land/[email protected]/assert/mod.ts";
import "./mocks/syscalls.ts";
import { aiSettings, getAndConfigureModel, initializeOpenAI, configureSelectedModel } from "./init.ts";
const settingsPageSample = `
Mock settings, yay
\`\`\`yaml
ai:
indexEmbeddings: true
indexEmbeddingsExcludePages:
- passwords
indexEmbeddingsExcludeStrings:
- foo
chat:
bakeMessages: false
customEnrichFunctions:
- enrichWithURL
textModels:
- name: gpt-4o
provider: openai
modelName: gpt-4o
- name: gemini-pro
modelName: gemini-pro
provider: gemini
baseUrl: https://api.gemini.ai/v1
secretName: GOOGLE_AI_STUDIO_KEY
imageModels:
- name: dall-e
provider: dalle
modelName: dall-e
embeddingModels:
- name: text-embedding-3-small
provider: openai
- name: ollama-all-minilm
modelName: all-minilm
provider: ollama
baseUrl: http://localhost:11434
requireAuth: false
\`\`\`
`;
const secretsPageSample = `
Mock secrets, yay
\`\`\`yaml
GOOGLE_AI_STUDIO_KEY: foo
OPENAI_API_KEY: bar
\`\`\`
`;
Deno.test("initializeOpenAI should set the system prompt correctly", async () => {
try {
await syscall("mock.setPage", "SETTINGS", settingsPageSample);
await syscall("mock.setPage", "SECRETS", secretsPageSample);
await initializeOpenAI();
assertEquals(
chatSystemPrompt.role,
"system",
"initializeOpenAI did not set the correct system prompt role",
);
assertEquals(
chatSystemPrompt.content.includes("interactive chat session with a user in a markdown-based note-taking tool called SilverBullet"),
true,
"initializeOpenAI did not set the correct system prompt content",
);
} catch (error) {
console.error(
"Error in test 'initializeOpenAI should set the system prompt correctly':",
error,
);
throw error;
}
});
Deno.test("configureSelectedModel should handle errors during API key retrieval", async () => {
try {
const model = {
name: "test-model",
provider: "test-provider",
modelName: "test-model",
requireAuth: true,
secretName: "INVALID_SECRET_NAME",
};
await syscall("mock.setPage", "SECRETS", secretsPageSample);
await assertThrowsAsync(
async () => {
await configureSelectedModel(model);
},
Error,
"Failed to read the AI API key. Please check the SECRETS page.",
"configureSelectedModel did not handle errors correctly",
);
} catch (error) {
console.error(
"Error in test 'configureSelectedModel should handle errors during API key retrieval':",
error,
);
throw error;
}
});
Deno.test("initializeOpenAI should configure models when multiple models are present", async () => {
try {
await syscall("mock.setPage", "SETTINGS", settingsPageSample);
await syscall("mock.setPage", "SECRETS", secretsPageSample);
await initializeOpenAI();
await getAndConfigureModel();
assertEquals(
aiSettings.textModels.length,
2,
"initializeOpenAI did not configure multiple text models correctly",
);
assertEquals(
aiSettings.imageModels.length,
1,
"initializeOpenAI did not configure multiple image models correctly",
);
assertEquals(
aiSettings.embeddingModels.length,
2,
"initializeOpenAI did not configure multiple embedding models correctly",
);
} catch (error) {
console.error(
"Error in test 'initializeOpenAI should configure models when multiple models are present':",
error,
);
throw error;
}
});
These tests ensure that:
- The
initializeOpenAI
function correctly sets up the system prompt. - The
configureSelectedModel
function handles errors during API key retrieval. - The
initializeOpenAI
function correctly configures models when multiple models are present.
Feel free to integrate these tests into your test suite.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Fetch the implementation of the initializeOpenAI function and other relevant functions from src/init.ts
ast-grep --lang typescript --pattern $'function initializeOpenAI($_) {
$$$
}' --pattern $'function getAndConfigureModel($_) {
$$$
}' --pattern $'async function configureSelectedModel($_) {
$$$
}' --pattern $'const aiSettings = $_' src/init.ts
Length of output: 413
Script:
#!/bin/bash
# Fetch the implementation of the initializeOpenAI function from src/init.ts
ast-grep --lang typescript --pattern $'function initializeOpenAI($_) {
$$$
}' src/init.ts
# Fetch the implementation of the getAndConfigureModel function from src/init.ts
ast-grep --lang typescript --pattern $'function getAndConfigureModel($_) {
$$$
}' src/init.ts
# Fetch the implementation of the configureSelectedModel function from src/init.ts
ast-grep --lang typescript --pattern $'async function configureSelectedModel($_) {
$$$
}' src/init.ts
# Fetch the declaration of the aiSettings constant from src/init.ts
ast-grep --lang typescript --pattern $'const aiSettings = $_' src/init.ts
Length of output: 4490
Summary by CodeRabbit
New Features
Bug Fixes
Tests