Make improvements to http_call to have better error handling

[ObsoleteXero]

xkcd comics are retrieved via .xkcd [number] now. Closes #554
.xkcd number [number] and .xkcd # [number] no longer work.

The uncaught error was caused by the failed response_object not being in the proper format for a json, so http_call() in base/data.py now returns a Munch with only the status code if the status code is not 200. Closes #553
http_call() now uses a Context Manager to handle the request.

[ajax146]

Just 200 is not always going to work.

An example from the github issue command requiring a 201:

status_code = response.get("status_code")
        if status_code != 201:
            await auxiliary.send_deny_embed(
                message=f"I was unable to create your issue (status code {status_code})",
                channel=ctx.channel,
            )
            return

[ajax146]

Additionally, other requests also don't use a 200 code, but just check by the existence of a different field, like strawpoll or spotify

[ObsoleteXero]

I changed it to return an empty dictionary if it fails to parse the response into a JSON. This ends up being a Munch with just the status code anyway but it's not based on the response status code anymore.

[TheKrol]

The changes in data broke other extensions

[ajax146]

As much as the async with parts work, and might even be better practice, indenting the majority of the code by 2 tabs makes it significantly harder to read.

Unless there is a significant advantage that I don't know of for using async with there, maintaining or increasing readability is important, and the old way seemed to work fine there. If there is an advantage, please do inform me of what it is

[ObsoleteXero]

As much as the async with parts work, and might even be better practice, indenting the majority of the code by 2 tabs makes it significantly harder to read.

Unless there is a significant advantage that I don't know of for using async with there, maintaining or increasing readability is important, and the old way seemed to work fine there. If there is an advantage, please do inform me of what it is

The advantage of using a Context Manager is that it will close the session when the execution of the code block stops. In the old way, an unhandled exception before reaching await client.close() would leave the session open. You can see this in the logs with non-existent xkcd comics for example.

2023-07-23 16:19:58.234 INFO: Command detected: +xkcd # 561651651
2023-07-23 16:20:01.891 INFO: Making HTTP GET request to URL: https://xkcd.com/561651651/info.0.json
2023-07-23 16:20:04.907 ERROR: Command error: Command raised an exception: ContentTypeError: 0, message='Attempt to decode JSON with unexpected mimetype: text/html; charset=utf-8', url=URL('https://xkcd.com/561651651/info.0.json')
2023-07-23 16:20:06.463 ERROR: Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x7f3bdafa6d90>

I was careless with my testing. Will fix the issues with other extensions.

Edit: I'm pretty sure in cases where the response object itself was returned to the callers, the response would never be closed unless the callers close it themselves.

[ajax146]

data.py line 220 should be logger.error, as well, you are logging an error. Using warning loses the stacktrace and any ability to debug a flaw in our logic and not just an external API error, and errors are logged differently and make it easier to see potential problems.

Other than that, this is looking good

[ajax146]

Additionally, doing any logging with cache_key needs to be done especially carefully, as they will contain API keys.
I am unsure if it is possible for that to leak API keys on Discord. I am going to have to carefully prove it cannot, as that could be a major problem

[ObsoleteXero]

Since http_call() already gets the URL including the API key, we could log with the root_url but that reduces the usefulness of the logs since it wouldn't log the parameters used in the call.
or we can try to manually remove the key using urllib.parse (Edit: regex might work better) using the parameter name for the API key/secret in the URL for each extension.

[ajax146]

No need to remove it, as the URLs are already logged as info just a few lines up.
I just really don't know how the logging system works in great detail, and can't figure out why that line doesn't send to either my DMs or the guild log channels. It's just something to be wary of, especially in that function, as it will get plaintext versions of almost all of the secrets