Show domain in oauth2 error log #109708
Show domain in oauth2 error log #109708
Conversation
home-assistant
bot
added
bugfix
cla-signed
core
small-pr
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| @@ -209,7 +209,10 @@ async def _token_request(self, data: dict) -> dict: | |||
| error_code = error_response.get("error", "unknown") | |||
| error_description = error_response.get("error_description", "unknown error") | |||
| _LOGGER.error( | |||
| "Token request failed (%s): %s", error_code, error_description | |||
| "Token request to %s failed (%s): %s", | |||
| self.token_url, | |||
There was a problem hiding this comment.
What I don't like is that the URL is printed first, error code/description last. The user might just skip over that.
I also don't know if the token URL is that important. It's usually bad redirect URI or client ID/secret.
There was a problem hiding this comment.
It's a way to pinpoint which integration this is causing it. My own prod currently struggles from the message shown in the description and I currently don't know which integration exactly is causing this. And there is a debug logger above this, so I can know this by enabling debug logging for homeassistant.helpers.config_entry_oauth2_flow, but that's an extra step. As in, when helping people, being able to pinpoint the integration in one logging entry is quite valuable.
There was a problem hiding this comment.
How about using self.domain ?
The URL is an implementation detail that the user generally does not know about.
I think the domain is generally more recognizable.
There was a problem hiding this comment.
Oh haven't checked if that's available
There was a problem hiding this comment.
I think the latest change to use the domain settles the comment.
ludeeus
changed the title
* Show token url in oauth2 error log * Fix tests * Use domain
Proposed change
Show token url in oauth2 error log. This would help pinpointing an issue to an integration.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: