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

Add payload size limit #1151

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Improve code quality
  • Loading branch information
yiwen101 committed Aug 13, 2024
commit cbd76332f7deafeaefbb5bac83e325d208c780a0
82 changes: 41 additions & 41 deletions lib/cadet_web/controllers/chat_controller.ex
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit: Can we combine the if into the with? See https://elixirforum.com/t/proper-way-of-handling-simple-booleans-in-with-statement/45556/4 for an example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing me to this. Fixed

Original file line number Diff line number Diff line change
Expand Up @@ -53,54 +53,54 @@ defmodule CadetWeb.ChatController do
response(200, "OK")
response(400, "Missing or invalid parameter(s)")
response(401, "Unauthorized")
response(421, "Message exceeds the maximum allowed length")
response(422, "Message exceeds the maximum allowed length")
response(500, "When OpenAI API returns an error")
end

def chat(conn, %{"conversationId" => conversation_id, "message" => user_message}) do
if String.length(user_message) > @max_content_size do
send_resp(
conn,
:unprocessable_entity,
"Message exceeds the maximum allowed length of #{@max_content_size}"
)
else
user = conn.assigns.current_user

with {:ok, conversation} <-
LlmConversations.get_conversation_for_user(user.id, conversation_id),
{:ok, updated_conversation} <-
LlmConversations.add_message(conversation, "user", user_message),
payload <- generate_payload(updated_conversation) do
case OpenAI.chat_completion(model: "gpt-4", messages: payload) do
{:ok, result_map} ->
choices = Map.get(result_map, :choices, [])
bot_message = Enum.at(choices, 0)["message"]["content"]

case LlmConversations.add_message(updated_conversation, "assistant", bot_message) do
{:ok, _} ->
render(conn, "conversation.json", %{
conversation_id: conversation_id,
response: bot_message
})

{:error, error_message} ->
send_resp(conn, 500, error_message)
end

{:error, reason} ->
error_message = reason["error"]["message"]
IO.puts("Error message from openAI response: #{error_message}")
LlmConversations.add_error_message(updated_conversation)
send_resp(conn, 500, error_message)
end
else
{:error, {:not_found, error_message}} ->
send_resp(conn, :not_found, error_message)
user = conn.assigns.current_user

{:error, error_message} ->
with true <- String.length(user_message) <= @max_content_size || {:error, :message_too_long},
{:ok, conversation} <-
LlmConversations.get_conversation_for_user(user.id, conversation_id),
{:ok, updated_conversation} <-
LlmConversations.add_message(conversation, "user", user_message),
payload <- generate_payload(updated_conversation) do
case OpenAI.chat_completion(model: "gpt-4", messages: payload) do
{:ok, result_map} ->
choices = Map.get(result_map, :choices, [])
bot_message = Enum.at(choices, 0)["message"]["content"]

case LlmConversations.add_message(updated_conversation, "assistant", bot_message) do
{:ok, _} ->
render(conn, "conversation.json", %{
conversation_id: conversation_id,
response: bot_message
})

{:error, error_message} ->
send_resp(conn, 500, error_message)
end

{:error, reason} ->
error_message = reason["error"]["message"]
IO.puts("Error message from openAI response: #{error_message}")
LlmConversations.add_error_message(updated_conversation)
send_resp(conn, 500, error_message)
end
else
{:error, :message_too_long} ->
send_resp(
conn,
:unprocessable_entity,
"Message exceeds the maximum allowed length of #{@max_content_size}"
)

{:error, {:not_found, error_message}} ->
send_resp(conn, :not_found, error_message)

{:error, error_message} ->
send_resp(conn, 500, error_message)
end
end

Expand Down
19 changes: 18 additions & 1 deletion test/cadet_web/controllers/chat_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,27 @@ defmodule CadetWeb.ChatControllerTest do
"message" => "#{message_exceed_length}"
})

assert response(conn, 422) ==
assert response(conn, :unprocessable_entity) ==
"Message exceeds the maximum allowed length of #{max_message_length}"
end

@tag authenticate: :student
@tag requires_setup: true
test "The content length less than the maximum allowed length but conversation belongs to another user",
%{conn: conn, conversation_id: conversation_id} do
assert conversation_id != nil
max_message_length = ChatController.max_content_length()
message_exceed_length = String.duplicate("a", max_message_length)

conn =
post(conn, "/v2/chats/#{conversation_id}/message", %{
"conversation_id" => conversation_id,
"message" => "#{message_exceed_length}"
})

assert response(conn, :not_found) == "Conversation not found"
end

@tag authenticate: :student
test "invalid conversation id", %{conn: conn} do
conversation_id = "-1"
Expand Down
Loading