Report a consumer the API no longer accepts

CHANGELOG.md

@@ -22,6 +22,7 @@
 
 ### Bug Fixes
 
+- Report a consumer the API no longer accepts as `%Discovergy.Error{reason: :consumer_rejected}`. `/oauth1/request_token` answers one with a `400` and an empty body, so `Discovergy.Client.reauthorize/3` failed with a bare `{:http_error, 400}` that nothing could tell apart from a malformed request. A long-running client that renews its token on a `401` would retry the dead consumer forever instead of registering a new one with `login/3`. `Exception.message/1` renders it, and `:not_logged_in`, as a sentence rather than an inspected atom.
 - Fix `Discovergy.Client.login/3` failing on a client that had already logged in. The consumer of the previous session was kept and used to sign the two requests that open the OAuth flow, which have to go out unsigned, so the API rejected them.
 - Fix `Discovergy.VirtualMeters.create_virtual_meter/3` sending a `GET`. Creating a virtual meter is a `POST`; the `GET` route expects a `meterId` and rejected the call. It also returns the new meter, which is now decoded into a `Discovergy.Meter`. `meterIdsMinus` is dropped when no meters are subtracted, as was intended.
 - Fix `Discovergy.Disaggregation.get_energy_by_device_measurements/4` returning measurements in an arbitrary order. They were sorted with `Date`, which only compares year, month and day, so every measurement of a day compared equal.

guides/api-quirks.md

@@ -54,8 +54,8 @@ to pre-empt with a timer.
 ## An expired token is a 401 with an empty body
 
 The API sends no body with it, so `Discovergy.Error` carries
-`reason: {:http_error, 401}` rather than a message. Match on the status, never
-on the reason:
+`reason: {:http_error, 401}` rather than a message. Match on the status rather
+than on that reason:
 
 ```elixir
 case Discovergy.Measurements.get_last_reading(client, meter_id) do
@@ -77,6 +77,50 @@ handler that re-authenticates on every 401 without backing off will hot loop
 against `authorize`, which is rate limited (see below). Back off from the
 second consecutive failure onwards.
 
+## Consumers expire too, and a rejected one is a 400
+
+The consumer registered by `consumer_token` is not permanent either, and it
+tends to go at the same time as the access token. A client that signed in the
+previous evening and reauthorized after the nightly expiry found the consumer
+gone.
+
+Where the data endpoints answer 401 for credentials they no longer accept,
+`/oauth1/request_token` answers 400 with an empty body:
+
+```
+$ curl -s -o /dev/null -w '%{http_code}\n' -X POST \
+    -H 'Authorization: OAuth oauth_consumer_key="<unknown>", ...' \
+    https://api.inexogy.com/public/v1/oauth1/request_token
+400
+```
+
+The same 400 comes back for a key the API knows but a signature that does not
+check out. The endpoint takes no parameters of its own, so a 400 there is about
+the consumer, not the request.
+
+This is the failure that strands a long-running client: `reauthorize/3` cannot
+get past `request_token`, and no backoff makes that better. Only `login/3`
+does, by registering a new consumer. Since a bare `{:http_error, 400}` does not
+say that, `Discovergy.Client.reauthorize/3` reports it as `:consumer_rejected`:
+
+```elixir
+case Discovergy.Client.reauthorize(client, email, password) do
+  {:ok, client} ->
+    client
+
+  {:error, %Discovergy.Error{reason: :consumer_rejected}} ->
+    Discovergy.Client.login(client, email, password)
+
+  {:error, error} ->
+    handle_error(error)
+end
+```
+
+`login/3` discards the credentials of the previous session, so it takes the
+same client. Registering once a night is far inside the `consumer_token` rate
+limit; registering on every 401 is not, which is why this stays a separate
+branch from token expiry.
+
 ## There is no credential-free refresh
 
 `/oauth1/authorize` takes the email and password directly as query parameters
@@ -110,6 +154,9 @@ calls `consumer_token`. The consumer can also be persisted and handed back to
 client = Discovergy.Client.new(consumer: consumer, token: token)
 ```
 
+A persisted consumer expires like any other, so be ready for the
+`:consumer_rejected` above and register a new one.
+
 `authorize` is limited more tightly than `consumer_token`. Two calls in quick
 succession from one address are enough to trigger it.
 

lib/discovergy/client.ex

@@ -6,6 +6,9 @@ defmodule Discovergy.Client do
   body. Use `reauthorize/3` rather than `login/3` to get a new one: the API
   rate limits consumer registration and asks clients to reuse tokens.
 
+  Consumers expire too. `reauthorize/3` then fails with
+  `reason: :consumer_rejected`, and `login/3` registers a new one.
+
   See [Quirks of the API](api-quirks.md) for the behaviour this library has to
   work around.
   """
@@ -98,6 +101,24 @@ defmodule Discovergy.Client do
   Named for what it does: the API has no credential-free refresh, so this needs
   the user's password just as `login/3` does. Only the consumer is spared.
 
+  ## When the consumer is gone
+
+  The API drops consumers too, usually at the nightly maintenance that expires
+  the token. `reauthorize/3` then fails with `reason: :consumer_rejected`, and
+  only `login/3` recovers from it:
+
+      case Discovergy.Client.reauthorize(client, email, password) do
+        {:error, %Discovergy.Error{reason: :consumer_rejected}} ->
+          Discovergy.Client.login(client, email, password)
+
+        result ->
+          result
+      end
+
+  `login/3` discards the credentials of the previous session, so hand it the
+  same client rather than a new one, which would lose its `:base_url` and
+  `:http_client`.
+
   ## Examples
 
       iex> {:ok, client} = Discovergy.Client.reauthorize(client, email, password)

lib/discovergy/error.ex

@@ -6,6 +6,13 @@ defmodule Discovergy.Error do
   client, or `{:http_error, status}` if the API replied with an unsuccessful
   status and an empty body. `:response` holds the raw HTTP response, or `nil`
   if the request never got that far.
+
+  Two reasons come from this library rather than from the API:
+
+  - `:not_logged_in` - `Discovergy.Client.reauthorize/3` was called on a client
+    that has no consumer, so there is nothing to reuse.
+  - `:consumer_rejected` - the API no longer accepts the consumer of the client
+    being reauthorized. Register a new one with `Discovergy.Client.login/3`.
   """
 
   alias Discovergy.HTTPClient
@@ -19,6 +26,8 @@ defmodule Discovergy.Error do
   @impl true
   def message(%__MODULE__{reason: reason}) when is_binary(reason), do: reason
   def message(%__MODULE__{reason: {:http_error, status}}), do: "HTTP #{status}"
+  def message(%__MODULE__{reason: :not_logged_in}), do: "not logged in"
+  def message(%__MODULE__{reason: :consumer_rejected}), do: "the consumer was rejected"
 
   def message(%__MODULE__{reason: %{__exception__: true} = reason}) do
     Exception.message(reason)

lib/discovergy/oauth.ex

@@ -76,9 +76,18 @@ defmodule Discovergy.OAuth do
   end
 
   defp get_request_token(client, consumer) do
-    with {:ok, body} <-
-           Client.post(client, "/oauth1/request_token", [], consumer: consumer, token: nil) do
-      {:ok, Token.into(URI.decode_query(body))}
+    case Client.post(client, "/oauth1/request_token", [], consumer: consumer, token: nil) do
+      {:ok, body} ->
+        {:ok, Token.into(URI.decode_query(body))}
+
+      # The endpoint takes no parameters of its own, so an empty-bodied 400 or
+      # 401 is about the consumer: an unknown key, or a signature that does not
+      # check out. A body means the API had something else to say.
+      {:error, %Error{reason: {:http_error, status}} = error} when status in [400, 401] ->
+        {:error, %Error{error | reason: :consumer_rejected}}
+
+      {:error, error} ->
+        {:error, error}
     end
   end
 

test/discovergy/error_test.exs

@@ -26,6 +26,13 @@ defmodule Discovergy.ErrorTest do
     assert Exception.message(error) == "HTTP 502"
   end
 
+  test "describes the reasons the library reports itself" do
+    assert Exception.message(%Discovergy.Error{reason: :not_logged_in}) == "not logged in"
+
+    assert Exception.message(%Discovergy.Error{reason: :consumer_rejected}) ==
+             "the consumer was rejected"
+  end
+
   test "reports a malformed response body", %{client: client} do
     mock(fn %{url: "https://api.inexogy.com/public/v1/meters"} ->
       {:ok, 200, [{"Content-Type", "application/json"}], "{"}

test/discovergy/oauth_test.exs

@@ -103,6 +103,36 @@ defmodule Discovergy.OAuthTest do
     refute auth =~ "oauth_token="
   end
 
+  for status <- [400, 401] do
+    @tag :logged_in
+    test "reports a consumer rejected with an empty #{status} as such", %{client: client} do
+      mock(fn %{url: "https://api.inexogy.com/public/v1/oauth1/request_token"} ->
+        {:ok, unquote(status), [], ""}
+      end)
+
+      assert {:error,
+              %Discovergy.Error{reason: :consumer_rejected, response: {unquote(status), [], ""}}} =
+               Discovergy.Client.reauthorize(client, "$email", "$password")
+    end
+  end
+
+  # Only the empty-bodied ones are the consumer. Anything the API bothered to
+  # explain keeps its explanation.
+  for {status, body} <- [
+        {400, "400 Bad Request: something else entirely"},
+        {429, "429 Too Many Requests: Rate of authorize requests is too high."}
+      ] do
+    @tag :logged_in
+    test "leaves a request_token #{status} that has a body alone", %{client: client} do
+      mock(fn %{url: "https://api.inexogy.com/public/v1/oauth1/request_token"} ->
+        {:ok, unquote(status), [{"content-type", "text/plain"}], unquote(body)}
+      end)
+
+      assert {:error, %Discovergy.Error{reason: unquote(body)}} =
+               Discovergy.Client.reauthorize(client, "$email", "$password")
+    end
+  end
+
   test "refuses to reauthorize a client that is not signed in", %{client: client} do
     assert {:error, %Discovergy.Error{reason: :not_logged_in}} =
              Discovergy.Client.reauthorize(client, "$email", "$password")