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

taxa: Handle broken API responses that are valid JSON #170

Open
synrg opened this issue Aug 8, 2022 · 0 comments
Open

taxa: Handle broken API responses that are valid JSON #170

synrg opened this issue Aug 8, 2022 · 0 comments
Assignees
Labels
bug Something isn't working inat 🍃 iNaturalist cog taxon iNaturalist taxa

Comments

@synrg
Copy link
Collaborator

synrg commented Aug 8, 2022

The error handling for API calls that return valid JSON, but don't contain expected fields in the response leaves something to be desired. For example, in inat.py in make_taxa_embed, where a taxon has already been retrieved, but the full taxon record is needed to get full means & status info:

        full_record = (
            await self.api.get_taxa(
                ctx, taxon.id, preferred_place_id=preferred_place_id
            )
        )["results"][0]
        full_taxon = Taxon.from_json(full_record)

There's no exception handling around this, potentially leading to a traceback like this:

[2022-08-08 14:57:01] [ERROR] red: Exception in command 'taxon'
Traceback (most recent call last):
  File "/home/redbot/.local/lib/python3.9/site-packages/discord/ext/commands/core.py", line 85, in wrapped
    ret = await coro(*args, **kwargs)
  File "/home/redbot/.local/share/Dronefly/cogs/CogManager/cogs/inatcog/commands/taxon.py", line 54, in taxon
    await self.send_embed_for_taxon(ctx, query_response)
  File "/home/redbot/.local/share/Dronefly/cogs/CogManager/cogs/inatcog/embeds/inat.py", line 1365, in send_embed_for_taxon
    embed=await self.make_taxa_embed(
  File "/home/redbot/.local/share/Dronefly/cogs/CogManager/cogs/inatcog/embeds/inat.py", line 1051, in make_taxa_embed
    full_record = (
TypeError: 'NoneType' object is not subscriptable

It's not clear why response["results"] is None. For no matches, the API should return this instead:

{
  "total_results": 0,
  "page": 1,
  "per_page": 30,
  "results": []
}

Since the command that was typed was ,t sharks from italy and that ought to have matched the taxon, this sort of failure should probably be handled as a retriable condition in api.py. Unfortunately, we don't know what the record actually looked like. However, we ought to be able to sanity-check the result, such as:

  • check if there is a "results" at all
  • if not, log the some reasonable fraction of the response (e.g. first 100 chars?) and raise a (custom?) exception to force a retry
    • it would be interesting to see what it is actually receiving! we might be able to make our final error message more useful if it repeatedly fails and on the final retry there's useful info contained within the response that we can pass back. but we won't know what that is until we catch it in the act in the logs.
  • don't forget to add the retry exception to the list of retriable exceptions if it isn't in it already

Finally, carefully check that commands like make_taxa_embed already have exception handling for the final exception raised if the underlying API call runs out of retries and raises a final LookupError (just as we recently did for search / show observation).

@synrg synrg added bug Something isn't working inat 🍃 iNaturalist cog taxon iNaturalist taxa labels Aug 8, 2022
@synrg synrg self-assigned this Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working inat 🍃 iNaturalist cog taxon iNaturalist taxa
Projects
None yet
Development

No branches or pull requests

1 participant