All-Hands-AI / OpenHands

🙌 OpenHands: Code Less, Make More
https://all-hands.dev
MIT License
37.61k stars 4.25k forks source link

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

Closed neubig closed 1 week ago

neubig commented 1 week ago

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

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
enyst commented 1 week ago

Just to note, also fixed here: https://github.com/All-Hands-AI/OpenHands/pull/5026

Along with a couple of other conversion issues.

neubig commented 1 week ago

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

enyst commented 1 week ago

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