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

Fix KeyError when accessing content in messages with function calls #5158

Closed
wants to merge 2 commits into from

Conversation

neubig
Copy link
Contributor

@neubig neubig commented Nov 21, 2024

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Fixes a "message['content'] key" error


Give a summary of what the PR does, explaining any non-trivial design decisions

Fixes #5142

The error occurs when a message in the messages list doesn't have a 'content' key. This can happen with OpenRouter when using function calls, as some messages might only have function call information without content.

Let me summarize what OpenHands did to fix the issue ("I" = OpenHands):

I identified that the error was occurring in the convert_fncall_messages_to_non_fncall_messages function when trying to access the 'content' key from messages that only had function calls.

I wrote a test case to reproduce the issue, which showed that messages with function calls but no content were causing a KeyError.

I fixed the issue by modifying the code to use message.get('content') instead of message['content'] in three places:

In the main message processing loop
In the system message handling
In the assistant message handling
The fix ensures that messages without a 'content' key are handled gracefully, which is important when dealing with function calls from OpenRouter or other providers that might not include content in their function call messages.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:3529795-nikolaik   --name openhands-app-3529795   docker.all-hands.dev/all-hands-ai/openhands:3529795

- Add test case to verify handling of messages without content
- Use dict.get() to safely access content key in messages
- Handle function call messages that don't include content
@neubig neubig requested a review from xingyaoww November 21, 2024 01:51
@neubig neubig mentioned this pull request Nov 21, 2024
1 task
@neubig neubig removed the lint-fix label Nov 21, 2024
@enyst
Copy link
Collaborator

enyst commented Nov 21, 2024

Just to note, also fixed here: #5026

Along with a couple of other conversion issues.

@neubig
Copy link
Contributor Author

neubig commented Nov 21, 2024

Oh, thanks a lot @enyst , I didn't see it mentioned in the issue. I'll close this in favor of yours.

@neubig neubig closed this Nov 21, 2024
@enyst
Copy link
Collaborator

enyst commented Nov 21, 2024

My bad, thank you. I'm working on a refactoring that will hopefully make these stop cropping up.

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.

[Bug]: message['content'] key error
3 participants