From b86ac58c7471a999aa03406dbbc664c22585293c Mon Sep 17 00:00:00 2001 From: Adrian Kumpf <8999358+adriankumpf@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:18:27 +0200 Subject: [PATCH] Report a consumer the API no longer accepts A long-running client renews its access token with reauthorize/3 when a data endpoint answers 401. That works until the consumer expires as well, which the API tends to do at the same nightly maintenance: from then on the renewal fails and no retry gets anywhere, because only a new consumer can, and registering one means calling login/3. There was no way to tell that apart. /oauth1/request_token answers a consumer it no longer accepts with a 400 and an empty body, so the failure arrived as a bare {:http_error, 400}, indistinguishable from a malformed request. Observed in production: an expired token at 03:52 followed by four hours of 400s, until a restart signed in from scratch. The step now reports it as reason: :consumer_rejected, which is the signal to call login/3. Only an empty body is mapped, so an error the API bothered to write keeps its message, and both 400 and 401 are covered because request_token takes no parameters of its own: the consumer key and its signature are the only thing it can refuse. Exception.message/1 gained clauses for :consumer_rejected and :not_logged_in, which rendered as inspected atoms until now. --- CHANGELOG.md | 1 + guides/api-quirks.md | 51 ++++++++++++++++++++++++++++++++-- lib/discovergy/client.ex | 21 ++++++++++++++ lib/discovergy/error.ex | 9 ++++++ lib/discovergy/oauth.ex | 15 ++++++++-- test/discovergy/error_test.exs | 7 +++++ test/discovergy/oauth_test.exs | 30 ++++++++++++++++++++ 7 files changed, 129 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44cf99d..cb22b1e 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/guides/api-quirks.md b/guides/api-quirks.md index 7f8e20f..c28f08a 100644 --- a/guides/api-quirks.md +++ b/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="", ...' \ + 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. diff --git a/lib/discovergy/client.ex b/lib/discovergy/client.ex index 08bd442..2439c57 100644 --- a/lib/discovergy/client.ex +++ b/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) diff --git a/lib/discovergy/error.ex b/lib/discovergy/error.ex index e1e15bf..3bc7cba 100644 --- a/lib/discovergy/error.ex +++ b/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) diff --git a/lib/discovergy/oauth.ex b/lib/discovergy/oauth.ex index e8b89cc..3b5447e 100644 --- a/lib/discovergy/oauth.ex +++ b/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 diff --git a/test/discovergy/error_test.exs b/test/discovergy/error_test.exs index 52c5cd6..abfa433 100644 --- a/test/discovergy/error_test.exs +++ b/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"}], "{"} diff --git a/test/discovergy/oauth_test.exs b/test/discovergy/oauth_test.exs index 0e7c20a..c06d20f 100644 --- a/test/discovergy/oauth_test.exs +++ b/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")