Skip to content

Commit

Permalink
Default the headers to Accept JSON
Browse files Browse the repository at this point in the history 
Fixes #594
Closes #596
  • Loading branch information
@timdorr
timdorr committed Jul 11, 2022
1 parent 89b3a22 commit 76fc78a
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/tesla_api/client.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def initialize(
retry_options: nil,
base_uri: nil,
sso_uri: nil,
client_options: {}
client_options: {headers: {"Accept" => "application/json"}}
)
@email = email
@base_uri = base_uri || BASE_URI
Expand Down

5 comments on commit 76fc78a

@distler
Copy link

Choose a reason for hiding this comment

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

Won't this break if the user supplies a client_options hash in TeslaApi::Client.new ?

@distler
Copy link

@distler distler commented on 76fc78a Jul 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Of course it will. With this commit in place,

tesla_api = TeslaApi::Client.new(client_options: {request: {timeout: 1000}})

fails with the same 406 error as before.

@distler
Copy link

Choose a reason for hiding this comment

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

If you really want to use the client_options hash, then you want to do something like

--- a/lib/tesla_api/client.rb	2022-07-11 00:38:20.000000000 -0500
+++ b/lib/tesla_api/client.rb	2022-07-11 03:12:42.000000000 -0500
@@ -15,7 +15,7 @@
       retry_options: nil,
       base_uri: nil,
       sso_uri: nil,
-      client_options: {headers: {"Accept" => "application/json"}}
+      client_options: {}
     )
       @email = email
       @base_uri = base_uri || BASE_URI
@@ -30,7 +31,7 @@

       @api = Faraday.new(
         @base_uri + "/api/1",
-        client_options
+        client_options.dup.merge({headers: {"Accept" => "application/json"}})
       ) { |conn|
         # conn.response :logger, nil, {headers: true, bodies: true}
         conn.request :json

@timdorr
Copy link
Owner Author

@timdorr timdorr commented on 76fc78a Jul 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

client_options are an escape hatch I added for those that needed it. If, for whatever reason, this needs to be changed to something else (application/json; charset=utf-8 maybe?) in the future, I'd rather still leave that door open for others. But the default here is reasonable, since most folks aren't using that option, or have already changed their invocation to add the header that's needed right now.

@distler
Copy link

Choose a reason for hiding this comment

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

Up to you, of course, but I'd prefer that code that worked before Tesla tweaked their API ought to work again, after this fix.

But you are right, the code I posted does not allow the user to override the value of the Accept header. This does:

--- a/lib/tesla_api/client.rb	2022-07-11 00:38:20.000000000 -0500
+++ b/lib/tesla_api/client.rb	2022-07-11 03:12:42.000000000 -0500
@@ -15,7 +15,7 @@
       retry_options: nil,
       base_uri: nil,
       sso_uri: nil,
-      client_options: {headers: {"Accept" => "application/json"}}
+      client_options: {}
     )
       @email = email
       @base_uri = base_uri || BASE_URI
@@ -30,7 +31,7 @@

       @api = Faraday.new(
         @base_uri + "/api/1",
-        client_options
+       {headers: {"Accept" => "application/json"}}.merge(client_options)
       ) { |conn|
         # conn.response :logger, nil, {headers: true, bodies: true}
         conn.request :json

Please sign in to comment.