twilio / twilio-ruby

A Ruby gem for communicating with the Twilio API and generating TwiML
MIT License
1.35k stars 462 forks source link

X-Twilio-Webhook-Enabled no longer works #669

Closed jplaisted closed 1 year ago

jplaisted commented 1 year ago

Issue Summary

As of 6.x; the X-Twilio-Webhook-Enabled header no longer works. My reading of the code would be that this applies to any other non standard header.

Steps to Reproduce

  1. Create a twilio chat client
  2. Get a reference to a channel
  3. Create a message with x_twilio_webhook_enabled set to true

Expected behavior: the underlying network headers in include 'X-Twilio-Webhook-Enabled'=>'true' Observed behavior: the underlying network headers do not include X-Twilio-Webhook-Enabled

Code Snippet

    channel.messages.create(
      body: body,
      from: from,
      attributes: attributes.to_json,
      x_twilio_webhook_enabled: "true",
    )

Technical details:

Root cause

I dug through the code. In 6.2.0 the request method here accepts a headers param; but ignores the passed in value and overrides it with standard headers. This is not the case with the pre-6.x code here.

So, more generically, the issue is that Twilio::REST::ClientBase#request ignores the headers parameter.

ehannes commented 1 year ago

I can verify that this is indeed a bug. I made the exact same POST request, once with the gem and once with a patched version of the gem. Here's the output.

Output for POST request for adding a message with version 6.3.1.

D, [2023-08-08T06:54:56.814378 #58076] DEBUG -- : --END TWILIO API REQUEST--
D, [2023-08-08T06:55:03.361042 #58076] DEBUG -- : --BEGIN Twilio API Request--
D, [2023-08-08T06:55:03.361254 #58076] DEBUG -- : Request Method: <POST>
D, [2023-08-08T06:55:03.361379 #58076] DEBUG -- : User-Agent:twilio-ruby/6.3.1 [filtered...] Ruby/3.2.1
D, [2023-08-08T06:55:03.361496 #58076] DEBUG -- : Accept-Charset:utf-8
D, [2023-08-08T06:55:03.361613 #58076] DEBUG -- : Content-Type:application/x-www-form-urlencoded
D, [2023-08-08T06:55:03.361724 #58076] DEBUG -- : Accept:application/json
D, [2023-08-08T06:55:03.361883 #58076] DEBUG -- : Host:conversations.twilio.com
D, [2023-08-08T06:55:03.361999 #58076] DEBUG -- : Path:/v1/Conversations/[filtered convresation id...]/Messages
D, [2023-08-08T06:55:03.362110 #58076] DEBUG -- : Query:
D, [2023-08-08T06:55:03.362229 #58076] DEBUG -- : Request Params:{}
D, [2023-08-08T06:55:04.011907 #58076] DEBUG -- : Response Status Code:201
D, [2023-08-08T06:55:04.012995 #58076] DEBUG -- : Response Headers:{the response headers...}

Output from my patched version of the gem:

D, [2023-08-08T06:57:18.919188 #58471] DEBUG -- : --END TWILIO API REQUEST--
D, [2023-08-08T06:57:28.296378 #58471] DEBUG -- : --BEGIN Twilio API Request--
D, [2023-08-08T06:57:28.296652 #58471] DEBUG -- : Request Method: <POST>
D, [2023-08-08T06:57:28.296806 #58471] DEBUG -- : User-Agent:twilio-ruby/6.3.1 [filtered...] Ruby/3.2.1
D, [2023-08-08T06:57:28.296957 #58471] DEBUG -- : Accept-Charset:utf-8
D, [2023-08-08T06:57:28.297113 #58471] DEBUG -- : Content-Type:application/x-www-form-urlencoded
D, [2023-08-08T06:57:28.297256 #58471] DEBUG -- : Accept:application/json
D, [2023-08-08T06:57:28.297401 #58471] DEBUG -- : X-Twilio-Webhook-Enabled:true
D, [2023-08-08T06:57:28.297586 #58471] DEBUG -- : Host:conversations.twilio.com
D, [2023-08-08T06:57:28.297795 #58471] DEBUG -- : Path:/v1/Conversations/[filtered conversation id...]/Messages
D, [2023-08-08T06:57:28.298068 #58471] DEBUG -- : Query:
D, [2023-08-08T06:57:28.298227 #58471] DEBUG -- : Request Params:{}
D, [2023-08-08T06:57:28.953662 #58471] DEBUG -- : Response Status Code:201
D, [2023-08-08T06:57:28.955040 #58471] DEBUG -- : Response Headers:{the response headers...}

@jplaisted was right about the solution as well. The code I added was .merge(headers) to actually use the passed headers:

# lib/twilio-ruby/base/client_base.rb
def request(host, port, method, uri, params = {}, data = {}, headers = {}, auth = nil, timeout = nil) # rubocop:disable Metrics/MethodLength
  auth ||= @auth
  headers = generate_headers(method).merge(headers) # added the .merge(headers)
ehannes commented 1 year ago

I can verify that the bug is fixed in main 😄 Would be great if you could release a new version of the gem now.

kridai commented 1 year ago

changes are released and are available in version 6.4.0

jplaisted commented 1 year ago

Thank you!