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

Bug fix: "IndexError: list index out of range" in langchain_openai/embeddings #29150

Closed
wants to merge 1 commit into from

Conversation

MartinChen1973
Copy link

@MartinChen1973 MartinChen1973 commented Jan 11, 2025

PR Title: langchain_openai: Improve error handling in embeddings/base.py


Description
This PR fixes a bug in the embeddings/base.py file where API-related errors were not handled properly. Specifically:

  • The old code raised an IndexError: list index out of range when the API key was expired or invalid.
  • This issue has been resolved by implementing proper error handling that raises user-friendly exceptions.

New behavior:

  • For an expired API key, it now raises:
    ValueError: Not enough available money, Please go to recharge.
  • For an invalid API key, it now raises:
    ValueError: Please check sk-************6GzDaH key from the platform and follow their guidelines.

This ensures clearer and more descriptive error messages for users, and potentially improves handling for other API-related issues.


Issue
No specific GitHub issue is referenced, but the PR addresses improper error handling in embeddings/base.py.


Dependencies
No additional dependencies are required for this change.


Testing Instructions
Please test the following code with the updated implementation:

from langchain_community.vectorstores import FAISS
from langchain_openai.embeddings import OpenAIEmbeddings
import os

os.environ['OPENAI_API_BASE'] = "https://api.agicto.cn/v1"
# Test with an expired key (out of money):
# os.environ['OPENAI_API_KEY'] = "***"

# Test with an invalid key:
os.environ['OPENAI_API_KEY'] = "sk-8Dxxl7SxhIHU8J3M6GzDaH"

vectorstore = FAISS.from_texts(
    [
        "some text",
        "some more text"
    ],
    embedding=OpenAIEmbeddings(),
)

Expected results:

  1. Expired key:
    Raises: ValueError: Not enough available money, Please go to recharge.
  2. Invalid key:
    Raises: ValueError: Please check sk-************6GzDaH key from the platform and follow their guidelines.

… 556

# The old code is not handling the error properly.
# It is giving an error: IndexError: list index out of range when the key is expired or wrong.
# Other api errors might also not be handled properly.

# Please test with the following code

# Old code gives error: IndexError: list index out of range
# New code gives error on expired key: ValueError: Not enough available money, Please go to  recharge
#            and error on wrong key: ValueError: Please check sk-************6GzDaH key from the  platform..

from langchain_community.vectorstores import FAISS
from langchain_openai.embeddings import OpenAIEmbeddings
import os

os.environ['OPENAI_API_BASE'] = "https://api.agicto.cn/v1"
# os.environ['OPENAI_API_KEY'] = "sk-8Dxxl7SxhIHU8J3M6GzDaHfzgUjC3Cm21BSR8Dxx4EuctMJ2" # Expired key, out of money
os.environ['OPENAI_API_KEY'] = "sk-8Dxxl7SxhIHU8J3M6GzDaH" # Wrong key

vectorstore = FAISS.from_texts(
    [
        "some text",
        "some more text"
    ],
    embedding=OpenAIEmbeddings(),
)
Copy link

vercel bot commented Jan 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 11, 2025 6:19am

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. Ɑ: embeddings Related to text embedding models module 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Jan 11, 2025
Copy link
Collaborator

@ccurme ccurme left a comment

Choose a reason for hiding this comment

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

I removed the API key from the PR description (the one that was used to demonstrate being out of funds). I'd consider it compromised.

This looks tailored to the https://api.agicto.cn/v1 API. Using the OpenAI API, you currently get informative errors (e.g., AuthenticationError). I don't think we want to change the error type in case users are catching them.

@MartinChen1973
Copy link
Author

@ccurme Oh I see. You have the point.

  1. The API key belongs to me and is out of date. But it's ok to delete it.
  2. Yes I agree with you that some messages from the LLM might include sensitive information. ( I once printed my Api key in the langchain model response.)

What if we do either of :

  1. Raise error saying "Calling to the Api failed with some error. Check if your key is invalid or other configurations.".
  2. Save a piece of "langchain standard log"(don't know if there is such kind of things) and ask the maintainer to check.

If you think one of them is ok I can help implement it. I was really bothered by the IndexError. It cost me 2 days to find out the truth. Thought it was about my venv. And I charged my account but did not notice that the key was already deleted (it's for students in a finished training. I teach langchain courses).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature Ɑ: embeddings Related to text embedding models module size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants