progval / Limnoria

A robust, full-featured, and user/programmer-friendly Python IRC bot, with many existing plugins.
https://docs.limnoria.net/
Other
622 stars 174 forks source link

getUrlFd: chain the original exception so that plugins can handle them further #1487

Closed jlu5 closed 3 years ago

jlu5 commented 3 years ago

Note: This uses a Python 3 specific syntax, which means that the minisix usage in utils.web probably isn't useful anymore.

A lot of APIs, including LastFM and GitHub, serve detailed API errors in the body text even if the request returned an error. This allows plugins to track HTTP errors returned from urllib and present these detailed messages (although the syntax to do so isn't the prettiest).

An example with my LastFM plugin:

diff --git a/LastFM/plugin.py b/LastFM/plugin.py
index de9087d..9bf1366 100644
--- a/LastFM/plugin.py
+++ b/LastFM/plugin.py
@@ -42,6 +42,7 @@ import supybot.log as log
 import supybot.ircdb as ircdb

 import json
+import urllib.error
 from datetime import datetime
 from .local import accountsdb

@@ -80,15 +81,20 @@ class LastFM(callbacks.Plugin):

         # see https://www.last.fm/api/show/user.getRecentTracks
         url = "%sapi_key=%s&method=user.getrecenttracks&user=%s&format=json" % (self.APIURL, apiKey, user)
+        self.log.debug("LastFM.np: url %s", url)
         try:
             f = utils.web.getUrl(url).decode("utf-8")
         except utils.web.Error as e:
-            irc.error(str(e), Raise=True)
-        self.log.debug("LastFM.np: url %s", url)
+            if isinstance(e.__cause__, urllib.error.HTTPError):
+                # When receiving an HTTP error, try reading and showing the API response anyways
+                # This requires a Limnoria version > 2021-08-01
+                f = e.__cause__.read().decode("utf-8")
+            else:
+               irc.error(str(e), Raise=True)

         data = json.loads(f)
         if "error" in data:
-            irc.error("%s: %s" % (data["error"], data.get("message")), Raise=True)
+            irc.error("LastFM API Error %s: %s" % (data["error"], data.get("message")), Raise=True)
         elif "recenttracks" not in data:
             irc.error("No recenttracks data found for %s" % user, Raise=True)

Before, the plugin would show generic errors like Error: HTTP Error 400: Bad Request. With this patch it can show the source error, like Error: LastFM API Error 10: Invalid API key - You must be granted a valid key by last.fm

jlu5 commented 3 years ago

From discussion on IRC, without this patch getting the implicitly chained error is also possible via __context__ instead of __cause__. But this is a fairly clear case of exception chaining, so we can be explicit about it. This also makes the traceback slightly clearer:

13:32:10 <+jlu5> implicit: During handling of the above exception, another exception occurred:
13:32:10 <+jlu5> explicit: The above exception was the direct cause of the following exception: