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

Use 'Content-Type' to access content type header #36

Merged
merged 2 commits into from
Mar 10, 2023
Merged

Use 'Content-Type' to access content type header #36

merged 2 commits into from
Mar 10, 2023

Conversation

matt-dutchie
Copy link
Contributor

Re: launchdarkly/ruby-server-sdk#210

We were receiving the following error when attempting to update the Ruby LD SDK up to version 7.0.2.

[2023-02-22T18:08:50] INFO  | [LDClient] Initializing stream connection
[2023-02-22T18:08:50] INFO  | Connecting to event stream at https://stream.launchdarkly.com/all
[2023-02-22T18:08:50] WARN  | Event source returned unexpected content type ''
...
# Times out, then retries ad nauseum

I was able to track down the error to the following line:

content_type = cxn.headers["content-type"]

content-type returns an empty string -- Content-Type returns the correct value.

The other lines changed in this PR are whitespace cleanup from my Ruby formatter.

@keelerm84
Copy link
Member

@matt-dutchie thank you for your contribution.

I'm surprised this change is necessary since the http gem seems to normalize header access (at least from reading through their code).

I'm trying to reproduce your issue to verify the fix but I haven't been successful yet. I saw on the SDK issue your dependencies are pinned to http 4.4.1 and faraday 1.10.0. Even with that I haven't been able to generate the same warning.

Do you know if your network setup is requiring any sort of redirects? Would you be able to provide a minimal reproduction case so I can verify the behavior?

I also noticed that the http gem provides a method content_type on the response object. What are your thoughts on using that instead of accessing the headers directly? Maybe this could help shield us from further issues of this type? 🤷

@matt-dutchie
Copy link
Contributor Author

@keelerm84 Yea this is likely something specific with our dependencies -- I've adjusted this PR to use the content_type method and it works, so that's probably a better solution than accessing the headers directly. Thanks for the response!

@@ -268,14 +268,14 @@ def connect
headers: build_headers
})
if cxn.status.code == 200
content_type = cxn.headers["content-type"]
content_type = cxn.content_type.mime_type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cxn.content_type returns a struct, mime_type gets us the appropriate string which matches the header.

@keelerm84 keelerm84 merged commit 27170f6 into launchdarkly:main Mar 10, 2023
@keelerm84
Copy link
Member

Fixed in 2.2.2

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

Successfully merging this pull request may close these issues.

2 participants