Skip to content
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

Added validation for OpenRouter API key #306

Merged
merged 5 commits into from Dec 18, 2023
Merged

Added validation for OpenRouter API key #306

merged 5 commits into from Dec 18, 2023

Conversation

kliu57
Copy link
Collaborator

@kliu57 kliu57 commented Dec 13, 2023

Closes #230

Added validation for OpenRouter API key

Current code (before PR):

  • No validation for OpenRouter API key

PR Code changes:

  • In src/lib/ai.ts, created new function validateApiKey() which validates both OpenAI and OpenRouter keys, by calling validateOpenAiApiKey() or validateOpenRouterApiKey()
  • In src/lib/ai.ts, create new function validateOpenRouterApiKey(), which uses response from https://openrouter.ai/docs#limits to check if OpenRouter API key is valid
  • In src/components/Message/AppMessage/Instructions.tsx, replaced call of validateOpenAiApiKey() with validateApiKey()
  • In src/components/PreferencesModal.tsx, replaced call of validateOpenAiApiKey() with validateApiKey()

Testing:

Invalid OpenRouter API key:

invalidOpenRouter1

invalidOpenRouter2

invalidOpenRouter3

Valid OpenRouter API key:

validOpenRouter

Regression Testing:

Invalid OpenAI API key:

invalidOpenAI1

invalidOpenAI2

invalidOpenAI3

Valid OpenAI API key:

validOpenAI

src/lib/ai.ts Outdated Show resolved Hide resolved
@tarasglek
Copy link
Owner

I like it, thank you for improving this!

@tarasglek
Copy link
Owner

tarasglek commented Dec 14, 2023

@kliu57 I invited you as a collaborator, I hope you accept. For all new work, please push as a branch to chatcraft.org repo, then we can benefit from cloudflare branch deployments.

@kliu57
Copy link
Collaborator Author

kliu57 commented Dec 14, 2023

@kliu57 I invited you as a collaborator, I hope you accept. For all new work, please push as a branch to chatcraft.org repo, then we can benefit from cloudflare branch deployments.

Sure, I can do that for future PRs. Do you need me to recreate my current PRs? (this one or PR#308)

@tarasglek
Copy link
Owner

@humphd ok to accept this one as is?

src/lib/ai.ts Outdated Show resolved Hide resolved
@kliu57 kliu57 requested a review from humphd December 15, 2023 05:30
Copy link
Collaborator

@humphd humphd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small things, this is looking good.

src/lib/ai.ts Outdated Show resolved Hide resolved
src/lib/ai.ts Outdated Show resolved Hide resolved
src/lib/ai.ts Outdated Show resolved Hide resolved
@kliu57
Copy link
Collaborator Author

kliu57 commented Dec 15, 2023

@humphd Made the three code fixes as suggested.

Also, I noticed in src/hooks/use-models.tsx there is a call to queryModels(). Now that my OpenRouter validation is not done inside of queryModels() anymore, this particular call to queryModels() will not be able to validate any OpenRouter keys. Is this something of concern?

@kliu57 kliu57 requested a review from humphd December 15, 2023 19:51
@humphd
Copy link
Collaborator

humphd commented Dec 18, 2023

@kliu57 This needs to be rebased to pick up the change we just landed in src/lib/ai.ts.

I think the call to queryModels() in src/hooks/use-models.tsx` doesn't need to be validated, since it will fail if the key is wrong anyway. We only really care about warning people and validating when we are setting the key during setup.

@humphd humphd merged commit 1905a0a into tarasglek:main Dec 18, 2023
3 checks passed
@humphd
Copy link
Collaborator

humphd commented Dec 18, 2023

Great work @kliu57. I hope you'll keep working ChatCraft in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use OpenRouter User Limits API to check key
3 participants