Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap validations in try/except so we can emit some logging #27

Closed
wants to merge 3 commits into from

Conversation

WebSpider
Copy link
Contributor

Fixes #22

@WebSpider WebSpider requested review from Prior99 and dvx76 and removed request for Prior99 September 23, 2024 12:54
@WebSpider
Copy link
Contributor Author

Conflicts with #31, let's see how we can catch validationerrors in mashu

@dvx76
Copy link
Member

dvx76 commented Sep 26, 2024

Conflicts with #31, let's see how we can catch validationerrors in mashu

We may not need to since mashu spits out failures like in skodaconnect/homeassistant-myskoda#34

@Prior99
Copy link
Contributor

Prior99 commented Sep 27, 2024

In the meantime I had added this:

    async def _make_get_request[T](self, url: str, deserialize: Callable[[str], T]) -> T:
        async with self.session.get(
            f"{BASE_URL_SKODA}/api{url}",
            headers=await self._headers(),
        ) as response:
            response_text = await response.text()
            try:
                data = deserialize(response_text)
            except Exception:
                _LOGGER.exception(
                    "Failed to load data from url %s. Return value was '%s'", url, response_text
                )
                raise
            else:
                return data

Which already wraps all get requests:

    async def get_charging(self, vin: str) -> Charging:
        """Retrieve information related to charging for the specified vehicle."""
        return await self._make_get_request(f"/v1/charging/{vin}", Charging.from_json)

Is this PR still needed?

If yes, can we find a more generic way?

@dvx76
Copy link
Member

dvx76 commented Sep 27, 2024

I think we can close this. If we still need more/better logs it will bubble up again while looking at issues.

@WebSpider WebSpider closed this Sep 27, 2024
@WebSpider WebSpider deleted the validation-logging branch October 9, 2024 14:02
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.

ValidationErrors should log the source json which failed to be parsed/validated
3 participants