twilio-ruby icon indicating copy to clipboard operation
twilio-ruby copied to clipboard

Docs missing for setting recommended timeouts

Open bf4 opened this issue 4 years ago • 7 comments

Issue Summary

We just got an 'execution expired' net open timeout from twilio (Twilio::REST::TwilioError) , which is fine, it happens, but because we're on Heroku, we need to limit requests to less than 30 seconds else they close it. I saw an error in the log and the customer said the request took a very long time (we validate phone numbers on some object creation using the lookup api).

Which got me to thinking:

  1. twilio-ruby should recommend shorter than default timeouts
  2. twilio-ruby should document how to set timeouts both globally and more specifically related to https://github.com/twilio/twilio-ruby/pull/467 https://github.com/twilio/twilio-ruby/issues/455

Steps to Reproduce

run an app for while and once in a blue moon get a server time out.

Code Snippet

globally maybe do

Faraday.options.timeout = 20       # open/read timeout in seconds
Faraday.options.open_timeout = 1.5 # connection open timeout in seconds

twilio specific would have something to do with the http client

Exception/Log

see related issues

Twilio::REST::TwilioError: execution expired (Most recent call first)
Faraday::ConnectionFailed: execution expired (Most recent call first)
Net::OpenTimeout: execution expired (Most recent call first)

Technical details:

  • twilio-ruby version: 5.31.1
  • ruby version: 2.6.2
  • environment: Heroku

bf4 avatar Feb 06 '20 23:02 bf4

Makes sense to me. Pull requests and +1s on the issue summary will help it move up the backlog.

childish-sambino avatar Feb 07 '20 14:02 childish-sambino

@childish-sambino thanks. I I couldn't figure out from a quick code read and poking at it how to configure the twilio http client globally

bf4 avatar Feb 10 '20 14:02 bf4

data point for whoever does this: https://github.com/ankane/the-ultimate-guide-to-ruby-timeouts/blob/master/test/twilio_test.rb

mlapping avatar Apr 30 '20 15:04 mlapping

seems related to https://github.com/twilio/twilio-ruby/pull/514/files https://github.com/twilio/twilio-ruby/pull/513 where the answer can be something like

client = Twilio::REST::Client.new(ENV["TWILIO_ACCOUNT_SID"], ENV["TWILIO_AUTH_TOKEN"]).tap do |c|
  if c.http_client.timeout.nil?
    c.http_client.instance_variable_set(:@timeout, 20)
    raise ArgumentError, "Expected to set timeout to 20" if c.http_client.timeout != 20
  end
end

bearing in mind that twilio doesn't distinguish the two timeout types and doesn't provide a writer for either

          f.options.open_timeout = request.timeout || @timeout
          f.options.timeout = request.timeout || @timeout

bf4 avatar Jun 11 '20 20:06 bf4

Hijacking my own issue since related

  • we're getting a Twilio::REST::TwilioError: execution expired from a timeout setting of 10 seconds
  • it's on message creation
  • per https://support.twilio.com/hc/en-us/articles/360007130274-Requirements-for-Connecting-to-the-Twilio-REST-API-and-Troubleshooting-Common-Issues we could wait 26 seconds to see if twilio will grace us with a 500 error and code https://www.twilio.com/docs/api/errors/20500 but I really don't want to wait that long. I'm pretty sure the extra 16 seconds won't improve things.
  • let's assume our situation is identical to the https://www.twilio.com/docs/api/errors/20500 case. The docs there read: "POST requests - sending an SMS or triggering an outbound call - are not idempotent. If you get a 500 Server Error on these requests, and you retry the request, it is possible for a customer to receive multiple messages or calls from your application."

So, I shouldn't assume I want to retry the request, no?

How long long to wait before I could reliably check the messages API for the absence of the sent text to mean it wasn't sent. Query by message id immediately after creating a message is basically immediately available, but query by number and date-sent are more likely to be 'eventually consistent', per CAP theorem

I have an open support ticket about this ( 4562575 ) but it's going back and forth so I thought I'd share the situation here in case it helps anyone (including myself)

bf4 avatar Jun 16 '20 21:06 bf4

What I ended up writing in my support ticket before closing it

  1. Twilio usually responds within a second
  2. We need requests to be quick because e.g. for phone number lookup we have to complete the request within 30 seconds due to heroku router constraints
  3. We've set the http client time out to 10 seconds which seems reasonable
  4. When we get an execution expired due to http client timeout on message send, it's possible that twilio eventually processed the request.
  5. We don't want to send the message twice.
  6. So, the issue I created is to try to handle this situation.. asked in terms of how long to wait before checking in the API if twilio subsequently sent the text nonetheless.

It seems to me right now that as long as we make transactional requests in a background worker, we want to let twilio timeout the connection and hence set the http client timeout to 30 seconds.

For regular API requests, like phone number validation, 10 seconds is fine. There's a huge unknown period of time between 10 and 30 (15 and 25 seconds) where twilio may or may not complete a transactional request and we'd be in better shape if twiio made that decision.

I think that having a timeout and transactional_timeout with sane defaults built in by Librarian would go a good way, even if they're opt-in.

Meanwhile, I've adjusted http timeouts in our workers to 28 seconds and kept it at 10 seconds for web dynos, though it's probably reasonable to reduce that to 2 seconds even

I can propose this in the twilio-ruby library, but probably it should be client-wide or at least in the docs, so that'd be Librarian, right? ref https://github.com/twilio/twilio-ruby/issues/493

I'll consider this closed as letting twilio close transactional requests seems to be the way to go.

My support rep responded to this with

I would just recommend making your timeout suggestions in the twilio-ruby library:

https://github.com/twilio/twilio-ruby/issues

so, ¯\_(ツ)_/¯ here's where I am

bf4 avatar Jul 05 '20 02:07 bf4

related? https://github.com/twilio/twilio-ruby/pull/559

bf4 avatar Jul 01 '21 19:07 bf4