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

invalid_request_error: 'content' is a required property #103

Closed
schneiderfelipe opened this issue Sep 6, 2023 · 3 comments · Fixed by #104
Closed

invalid_request_error: 'content' is a required property #103

schneiderfelipe opened this issue Sep 6, 2023 · 3 comments · Fixed by #104
Labels
bug Something isn't working

Comments

@schneiderfelipe
Copy link
Contributor

Hi @64bit!

TL;DR: async-openai seems to silently create unexpected, invalid requests because the OpenAI API expects the content field be present, even if it is null.

The content field seems to be required in situations that, AFAIK, is only mentioned en passant in the OpenAI API reference documentation:

content (string or null) Required
The contents of the message. content is required for all messages, and may be null for assistant messages with function calls.

(Emphases mine.) As such, I don't know if this should be addressed in async-openai. Depending on how you interpret the documentation, it might be the case that async-openai is in fact violating the API contract. In any case, I had such a bad time debugging that I think it's worth creating an issue.

The issue

The following fails by using curl directly:

$ curl https://api.openai.com/v1/chat/completions -u :$OPENAI_API_KEY -H 'Content-Type: application/json' -d '{"model":"gpt-3.5-turbo","messages":[{"role":"user","content":"What is the weather like in Boston?"},{"role":"assistant","function_call":{"name":"get_current_weather","arguments":"{\"location\":\"Boston, MA\"}"}},{"role":"function","content":"{\"forecast\":[\"sunny\",\"windy\"],\"location\":\"Boston, MA\",\"temperature\":\"72\",\"unit\":null}","name":"get_current_weather"}],"functions":[{"name":"get_current_weather","description":"Get the current weather in a given location","parameters":{"properties":{"location":{"description":"The city and state, e.g. San Francisco, CA","type":"string"},"unit":{"enum":["celsius","fahrenheit"],"type":"string"}},"required":["location"],"type":"object"}}],"temperature":0.0,"max_tokens":null}'
{
  "error": {
    "message": "'content' is a required property - 'messages.1'",
    "type": "invalid_request_error",
    "param": null,
    "code": null
  }
}

The second message {"role":"assistant","function_call":{"name":"get_current_weather","arguments":"{\"location\":\"Boston, MA\"}"}} does not contain a content field. The solution is to insert one. Both "content":null and "content":"" work. So there's a difference between omitting the content field and having "content":null.

Why

I created the failing example above by serializing a CreateChatCompletionRequest directly. The issue lies in skipping serializing the content field if it is None:

/// The contents of the message.
/// `content` is required for all messages except assistant messages with function calls.
#[serde(skip_serializing_if = "Option::is_none")]
pub content: Option<String>,

The current workaround is to set content to an empty string:

aot::ChatCompletionRequestMessageArgs::default()
    .role(aot::Role::Assistant)
    .function_call(aot::FunctionCall { name, arguments })
    .content("")
    .build()?

Again, depending on how you interpret the OpenAI API documentation, this behaviour might break the API contract. What do you think?

@64bit
Copy link
Owner

64bit commented Sep 7, 2023

Hi @schneiderfelipe - Thank you for a detailed description of the issue.

It does sound like the issue is showstopper and it makes sense to fix this in async-openai and make it work with current behavior of OpenAI API.

So the fix is to remove #[serde(skip_serializing_if = "Option::is_none")] attribute from content field?

@64bit 64bit added the bug Something isn't working label Sep 7, 2023
@schneiderfelipe
Copy link
Contributor Author

Yes, I think so, at least for the presented use case. This makes me sad actually, the current behaviour of async-openai feels like the right one, but for no reason OpenAI wants to make a distinction between the absence of a content field and the presence of "content":null.

@64bit 64bit mentioned this issue Sep 8, 2023
@64bit 64bit closed this as completed in #104 Sep 8, 2023
@64bit
Copy link
Owner

64bit commented Sep 8, 2023

This fix is released in v0.14.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants