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

Pagerduty /schedules endpoint not allowing GETs with a BODY. #56

Closed
ag-TJNII opened this issue Jan 16, 2024 · 2 comments
Closed

Pagerduty /schedules endpoint not allowing GETs with a BODY. #56

ag-TJNII opened this issue Jan 16, 2024 · 2 comments

Comments

@ag-TJNII
Copy link
Contributor

We had an outage in our app that uses this gem late last week, that looks up Pagerduty schedules. The original exception was:

/usr/local/lib/ruby/3.0.0/json/common.rb:216:in `parse': 809: unexpected token at '' (JSON::ParserError)
	from /usr/local/lib/ruby/3.0.0/json/common.rb:216:in `parse'
	from /usr/local/bundle/gems/pager_duty-connection-2.2.0/lib/pager_duty/connection.rb:44:in `call'
	from /usr/local/bundle/gems/faraday_middleware-1.2.0/lib/faraday_middleware/response_middleware.rb:36:in `call'
	from /usr/local/bundle/gems/faraday-1.10.3/lib/faraday/middleware.rb:18:in `call'
	from /usr/local/bundle/gems/faraday-1.10.3/lib/faraday/middleware.rb:18:in `call'
	from /usr/local/bundle/gems/faraday_middleware-1.2.0/lib/faraday_middleware/request/encode_json.rb:26:in `call'
	from /usr/local/bundle/gems/pager_duty-connection-2.2.0/lib/pager_duty/connection.rb:76:in `call'
	from /usr/local/bundle/gems/faraday-1.10.3/lib/faraday/middleware.rb:18:in `call'
	from /usr/local/bundle/gems/faraday-1.10.3/lib/faraday/rack_builder.rb:154:in `build_response'
	from /usr/local/bundle/gems/faraday-1.10.3/lib/faraday/connection.rb:516:in `run_request'
	from /usr/local/bundle/gems/pager_duty-connection-2.2.0/lib/pager_duty/connection.rb:218:in `run_request'
	from /usr/local/bundle/gems/pager_duty-connection-2.2.0/lib/pager_duty/connection.rb:197:in `get'
[Snip app specific trace]

To debug I installed the branch from #55 (👍 on that PR, by the way), and zeroed in on the real error:

PagerDuty::Connection::ApiError: Got HTTP 500: Internal Server Error
From https://api.pagerduty.com/schedules/[Schedule ID]?limit=100&offset=0&since=2024-01-11T19%3A00%3A00%2B00%3A00&until=2024-01-11T21%3A00%3A00%2B00%3A00

In engaging Pagerduty support I received the following response:

Due to some changes that have been rolled out, the /schedules endpoint stopped allowing GET requests that include a body; your recent GET requests that include a body have been receiving a 500 error response.

Our engineering team is currently working to reinstate the previous behavior; however, if you'd like to circumvent the issue in the meantime, we'd recommend sending GET requests to the /schedules endpoint without the inclusion of a request body, which is a generally recommended practice.

I took a look at the PagerDuty Connection code I think this might be due to the default body {} argument value in PagerDuty::Connection.run_request:

def run_request(method, path, body: {}, headers: {}, query_params: {})

Looking at the upstream Faraday docs this argument should be a string or nil.

https://www.rubydoc.info/github/lostisland/faraday/Faraday%2FConnection:run_request

I assume that hash is being converted to a string somewhere in the Faraday code. However, for get shouldn't it be nil? I'm wondering if get needs a different default than post and put.

@ag-TJNII
Copy link
Contributor Author

Proposed patch in #57, apologies for the commit noise. I forgot Github will notify on commit messages.

@ag-TJNII
Copy link
Contributor Author

Performing a little more debugging {} vs nil does appear to be the issue.

When the body argument to Faraday run_request is {}:

connection = Faraday.new
connection.run_request(:get, 'http://127.0.0.1:8765', {}, {})
$ nc -l -p 8765
GET / HTTP/1.1
User-Agent: Faraday v2.7.10
Content-Type: application/x-www-form-urlencoded
Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
Accept: */*
Host: 127.0.0.1:8765
Content-Length: 0

Whereas when it is nil:

connection = Faraday.new
connection.run_request(:get, 'http://127.0.0.1:8765', nil, {})
$ nc -l -p 8765
GET / HTTP/1.1
User-Agent: Faraday v2.7.10
Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
Accept: */*
Host: 127.0.0.1:8765

Note that when nil is passed there is no Content-Type or Content-Length header. The proposed patch in #57 should resolve this.

technicalpickles added a commit that referenced this issue Jan 16, 2024
Set HTTP GET body to nil, resolves #56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant