Skip to content

fix: token refresh - #835

Open
timhae wants to merge 1 commit into
openviess:masterfrom
timhae:feature/tharing/fix-token-refresh
Open

timhae wants to merge 1 commit into
openviess:masterfrom
timhae:feature/tharing/fix-token-refresh

Conversation

@timhae

@timhae timhae commented Oct 3, 2026

Copy link
Copy Markdown

without this change the token refresh fails with

2026-10-03 15:49:08,216 :: INFO :: vicare_exporter :: Fetching metrics
Traceback (most recent call last):
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/PyViCare/PyViCareAbstractOAuthManager.py", line 47, in get
    raw_response = self.__oauth.get(f"{API_BASE_URL}{url}", timeout=31)
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/requests/sessions.py", line 671, in get
    return self.request("GET", url, params=params, **kwargs)
           ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/integrations/requests_client/oauth2_session.py", line 140, in request
    return super().request(method, url, auth=auth, **kwargs)
           ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/requests/sessions.py", line 635, in request
    prep = self.prepare_request(req)
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/requests/sessions.py", line 541, in prepare_request
    p.prepare(
    ~~~~~~~~~^
        method=method.upper(),
        ^^^^^^^^^^^^^^^^^^^^^^
    ...<10 lines>...
        hooks=merge_hooks(request.hooks, self.hooks),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/requests/models.py", line 443, in prepare
    self.prepare_auth(auth, url)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/requests/models.py", line 689, in prepare_auth
    r = auth_handler(self)
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/integrations/requests_client/oauth2_session.py", line 25, in __call__
    self.ensure_active_token()
    ~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/integrations/requests_client/oauth2_session.py", line 22, in ensure_active_token
    raise InvalidTokenError()
authlib.integrations.base_client.errors.InvalidTokenError: token_invalid:

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/wsgiref/handlers.py", line 137, in run
    self.result = application(self.environ, self.start_response)
                  ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/prometheus_client/exposition.py", line 158, in prometheus_app
    status, headers, output = _bake_output(registry, accept_header, accept_encoding_header, params, disable_compression)
                              ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/prometheus_client/exposition.py", line 122, in _bake_output
    output = encoder(registry)
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/prometheus_client/openmetrics/exposition.py", line 61, in generate_latest
    for metric in registry.collect():
                  ~~~~~~~~~~~~~~~~^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/prometheus_client/registry.py", line 99, in collect
    yield from collector.collect()
  File "/home/tim/repos/vicare-exporter/vicare_exporter/metrics.py", line 59, in collect
    for installation_id, features in self._fetch_features().items():
                                     ~~~~~~~~~~~~~~~~~~~~^^
  File "/home/tim/repos/vicare-exporter/vicare_exporter/metrics.py", line 119, in _fetch_features
    str(device.accessor.id): device.service.fetch_all_features(
                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        device.accessor
        ^^^^^^^^^^^^^^^
    )
    ^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/PyViCare/PyViCareService.py", line 70, in fetch_all_features
    return self.oauth_manager.get(url)
           ~~~~~~~~~~~~~~~~~~~~~~^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/PyViCare/PyViCareAbstractOAuthManager.py", line 61, in get
    self.renewToken()
    ~~~~~~~~~~~~~~~^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/PyViCare/PyViCareBrowserOAuthManager.py", line 113, in renewToken
    self.oauth_session.refresh_token(TOKEN_URL, refresh_token=refresh_token)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/oauth2/client.py", line 276, in refresh_token
    body = prepare_token_request(
        "refresh_token", body, refresh_token=refresh_token, **kwargs
    )
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/oauth2/rfc6749/parameters.py", line 107, in prepare_token_request
    return add_params_to_qs(body, params)
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/common/urls.py", line 88, in add_params_to_qs
    return url_encode(qs)
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/common/urls.py", line 24, in url_encode
    encoded.append((to_bytes(k), to_bytes(v)))
                                 ~~~~~~~~^^^
  File "/nix/store/9f8pr2v6g39yfivpm32afmv6mz8dpi48-python3-3.14.7-env/lib/python3.14/site-packages/authlib/common/encoding.py", line 15, in to_bytes
    return bytes(x)
TypeError: cannot convert 'method' object to bytes

@lackas

lackas commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

The fix is right: self.oauth_session.refresh_token is the method, not the token, so every refresh has failed since #302, and a mocked OAuth2Session with a token dict reproduces the TypeError on master. Authlib falls back to self.token["refresh_token"] on its own, so self.oauth_session.refresh_token(TOKEN_URL) would do the same. A regression test along those lines would be good, and separately the renewed token is never written back to token_file, so after a restart it refreshes with the old refresh token again.

@timhae
timhae force-pushed the feature/tharing/fix-token-refresh branch from 1a672de to 36a5ebe Compare October 5, 2026 12:53
@timhae

timhae commented Oct 5, 2026

Copy link
Copy Markdown
Author

I simplified the fix according to your comment and added an LLM generated test, please double check as I am not very familiar with all involved code bases, the check looks good to me though. I am not sure about the storing to file aspect, should I do anything about that?

@lackas

lackas commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Thanks, looks good: both tests fail on master and pass with the fix. The token file part is separate, I'll open an issue for it once this is merged, so nothing more needed here.

@timhae

timhae commented Oct 18, 2026

Copy link
Copy Markdown
Author

Do you need anything from me to merge this?

@lackas

lackas commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

@timhae Nothing more needed from you. The workflow runs still wait for a maintainer to approve them, since this is your first PR here, and merging is up to @CFenner.

This branch has not been deployed

No deployments
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