cyberchitta / openai_ex

Community maintained Elixir library for OpenAI API
https://hexdocs.pm/openai_ex
Apache License 2.0
139 stars 19 forks source link

Hitting Finch Timeout #48

Closed gworkman closed 10 months ago

gworkman commented 1 year ago

Describe the bug

I'm getting the following timeout error while making requests to the Chat Completions endpoint.

** (Mint.TransportError) timeout
    (finch 0.16.0) lib/finch.ex:359: Finch.request!/3
    (openai_ex 0.2.0) lib/openai_ex/http.ex:65: OpenaiEx.Http.finch_run/1

Can we add some timeout options in the application config? Finch.request!/3 supports the keyword options :pool_timeout and :receive_timeout.

Note: this only happens about 5-10% of the time, and usually only with longer requests (1500+ tokens). Retrying the request usually completes within 1-2 seconds.

Also not sure if it is a factor, but I'm located in Europe and OpenAI only has US servers for now it seems

To Reproduce

Run (and/or add small variations) until it times out

prompt = [
  OpenaiEx.ChatMessage.system(
    "Generate a news article in the style of CNN on the topic provided"
  ),
  OpenaiEx.ChatMessage.user(
    "Kia and Hyundai cars being stolen because of security flaw in locking mechanism"
  ),
  OpenaiEx.ChatMessage.assistant("""
  You may have read about the recent lawsuits against Kia and Hyundai in New York and other US cities. But what exactly led us to this point? As with many viral topics these days, it all started on TikTok.
  During the pandemic, bored and tech-savvy teenagers began posting videos online showing how to steal Kias and Hyundais. Thefts of these cars began increasing dramatically around the country, and soon the social media challenge—where people film themselves doing something (like dumping a bucket of icy water over their head) then post the video online as proof—had caused deadly car crashes, a class-action lawsuit, and even drops in the automakers’ stock prices. Kia and Hyundai both began offering software updates and recalling cars, but the damage was done.

  The Kia Boys & the TikTok challenge
  The COVID-19 pandemic led to a huge number of social media challenges, especially among kids, simply due to boredom and extra time. In late 2021, the “Kia Boys” began going viral on TikTok. They were participating in a social media challenge that involved stealing Kias and Hyundais using a specific method and posting the results on TikTok, Snapchat, and YouTube.
  These teenagers, who have stolen tens of thousands of cars, are mostly from Milwaukee and Columbus, Ohio, although the trend has since spread nationwide. The topic has even made it into songs, with rappers bragging about stealing Kias.
  The Kia Boys are not an official organization but rather a general collective of teenagers who have used a particular hack to steal cars. Many are too young to even have a driver's license. They don’t do it for money or even to keep the cars—they do it for fun, for the joy rides.
  Not only are these teenagers stealing cars and sharing the videos on social media, but they are driving the cars recklessly, causing property damage and fatal crashes. The thieves usually abandon the cars, often totaled and filled with empty bottles of alcohol.
  At least one arrest has been made in connection to these crimes. In June 2022, Markell Hughes was arrested in Milwaukee after crashing a stolen red Hyundai. Police were able to match the license plate of the vehicle to one that had been reported stolen. They also identified Hughes and the vehicle in a YouTube video called “Kia Boys Documentary (A Story of Teenage Car Theft),” which now has over 6 million views.

  What makes these cars so easy to steal?
  The security flaw that has led to this issue stems from Kia’s cost-saving measures and a lack of regulations. In Canada, for example, cars made from September 2007 onward are required to have immobilizers—a common anti-theft device that prevents a car from being started without the smart key present. But this regulation doesn’t exist in the United States.
  The Kia Boys can easily steal Kia models manufactured from 2011-2021 by accessing the steering wheel column. The method is probably easier than you think: all the Kia Boys need to do is break a window, pry open the steering column, and plug a charging cord onto a small metal nub beneath the ignition cylinder, which is perfectly sized to fit into a USB-A plug. Then just twist the plug and the car starts.
  In Milwaukee and Columbus, where the phenomenon is most prominent, you’ll now see most Kias sporting a steering wheel lock. Kia and Hyundai dealerships in these cities are reportedly overwhelmed with all of the damaged cars that owners are finding abandoned. It’s even causing a shortage of parts.

  The data
  With the concentration of the Kia thieves being in Milwaukee, we can already see the impact there.
  In 2019, the Milwaukee Police Department reported roughly 3,500 vehicles stolen, with Kias and Hyundais making up only 6 percent. In 2021, the number of stolen cars in Milwaukee increased by nearly 200 percent to 10,500. Of those, 67 percent were Kias or Hyundais. This means that the number of Kias and Hyundais stolen in Milwaukee increased by more than 3,000 percent in two years—this was not a small operation.
  The majority of these Milwaukee car thefts have been happening in the Kilbourn Town/Westown neighborhood, where around 1,000 vehicles were stolen in the last year. This neighborhood already ranked sixth for most Milwaukee car crashes in 2018–2022.
  The trend is not limited to its Milwaukee origins and has spread to cities nationwide. In August 2022, Chicago officials reported an 800 percent increase in Kia and Hyundai thefts compared to the previous year.

  Automakers settle class-action lawsuit
  The Kia and Hyundai thefts have most recently made the news due to a $200 million settlement being reached. New York City recently sued both of the South Korean automakers for causing a “public nuisance,” according to Manhattan federal court. The lawsuit alleges that by failing to install devices that prevent their cars from being stolen, Kia and Hyundai are enabling the epidemic of car thefts.
  The class-action lawsuit was settled in May 2023, with Hyundai and Kia being ordered to pay out $200 million, compensating roughly 9 million people for their losses. Additional lawsuits are being filed by other cities, including Baltimore, Seattle, Milwaukee, and St. Louis.
  Both automakers have responded to the recent bad press by making safety improvements. Hyundai has promised that all vehicles manufactured after November 2021 now come with ignition immobilizers, and it began selling a security kit that purportedly targets the method of theft being used. Both Kia and Hyundai also released a free theft deterrent software update to all vehicle owners in February 2023. Kia also responded by clarifying that most of its current models come with a key fob and push-button start, which makes the cars harder to steal. For those who own older models with a mechanical key, Kia and Hyundai owners can protect their cars by purchasing steering wheel locks.
  Social media companies have also begun addressing their role in the situation. TikTok claims it does not allow videos that promote car theft and will remove them, but it’s not that simple. There is a gray area of videos that don’t necessarily show the crime and therefore aren’t breaking any rules.
  """),
  OpenaiEx.ChatMessage.user("Elon Musk fires another 40% of Twitter Staff")
]

System.get_env("LB_OPENAI_API_KEY")
|> OpenaiEx.new()
|> OpenaiEx.ChatCompletion.create(%{model: "gpt-3.5-turbo-0613", messages: prompt})

Code snippets

No response

OS

Mac

Elixir version

1.14.5

Library version

v0.2.0

restlessronin commented 1 year ago

@gworkman for the time being i would prefer not to add configuration. I would rather set the default timeout to a high enough value that all normal use-cases are addressed. WDYT about doubling the default Finch timeout from 15s to 30s? Or I could even raise it to 60s.

restlessronin commented 1 year ago

@gworkman I increased the default receive timeout to 45 seconds, and have published to hex.pm as v 0.2.1. can you check that this resolves your problem?

gworkman commented 1 year ago

Works for me, thank you! I will check today and let you know :)

restlessronin commented 1 year ago

@gworkman great. please let me know if it works, so I can close this issue.

gworkman commented 1 year ago

Good news! Extending the timeout works great, it is always returning some response now. Bad news: the problem is with OpenAI. I have been getting these responses from the endpoint after calling OpenaiEx.ChatCompletion.create/2:

%{"error" => %{"code" => nil, "message" => "That model is currently overloaded with other requests. You can retry your request, or contact us through our help center at help.openai.com if the error persists. (Please include the request ID in your message.)", "param" => nil, "type" => "server_error"}}

Fortunately, I didn't really need to change my code - I have been running these calls in Task.Supervisor with the restart: :transient option enabled for each Task. When I get the error, it automatically retries for me. Idk if we want to handle this differently in the future though

Happy to contribute if you give me some guidance! :)

restlessronin commented 1 year ago

@gworkman I like the good news :-)

For the moment, I'd like to keep the library as a very thin wrapper on the OpenAI REST API. So I'd prefer not to "fatten" the client by adding support at this level (at least not yet).

What would be immediately useful would be sample code (hopefully as a Livebook) which shows how you're handling the error / retries, etc, so all users are aware of problematic scenarios and how to approach them.

If you contribute a sample "Error Handling" livebook, i'd be happy to add it to the docs.

SinKasula commented 10 months ago

@restlessronin I am seeing the Mint Timeout error using the gpt-4 turbo model. It's taking more than 2 minutes to respond when the token size is greater than 1500. With the token size being 128K for the latest models, this is an occurrence 7/10 times for my use case. Could you please make it configurable as suggested above? or Change the receive_timeout to 10 minutes? Thank you!

restlessronin commented 10 months ago

@SinKasula I have made the timeout configurable using with_receive_timeout and left the default at 120_000. While the intention was to cover 99% of normal usage, I'm not comfortable having the default be 10 minutes.

I have also added documentation covering configuration to the user guide.

Changes should already be available on hex as v0.5.2.

restlessronin commented 10 months ago

@SinKasula can you confirm that the fix solved your problem? Thanks

hubertlepicki commented 10 months ago

@restlessronin are you still against providing configuration for Finch?

I would love to tweak some of the connection options too and I don't see a way to do it. Having a glance at the code, I think the way to do it would be to hook into this line:

https://github.com/restlessronin/openai_ex/blob/669c663a557ab691b6b8b01c51790c2984ad716b/lib/openai_ex/application.ex#L9C6-L9C6

we could grab the options from either Config and/or specify a callback function in config that is called to fetch the configuration - I don't mind either solution.

SinKasula commented 10 months ago

@restlessronin The above fix did not work. I think the options are not being set on the Finch request.

Screenshot 2024-01-02 at 10 04 35 AM
restlessronin commented 10 months ago

@SinKasula My bad. I made an incorrect assumption about Finch.build options. Should be fixed now in v0.5.4 on hex. I've tested it, but please confirm anyway.

restlessronin commented 10 months ago

@hubertlepicki I moved your comment and my response to a new issue

SinKasula commented 10 months ago

@restlessronin Thank you! This solved my problem