sedrubal / brother_printer_fwupd

Script to update the firmware of some Brother printers
https://pypi.org/project/brother_printer_fwupd/
GNU General Public License v3.0
30 stars 5 forks source link

Exception swallowed and thus hides real bug #1

Closed frioux closed 3 years ago

frioux commented 3 years ago

I tried this on my MFC-J430W and got an error:

[i] Querying printer info via SNMP.
[i]   Detected MFC-J430W with 1 firmware parts.
[i] Querying firmware download URL from brother update API.
[!] Did not receive any url.
[!] Maybe the firmware is already up to date or there is a bug.
[!] This is the response of brothers update API:
<?xml version="1.0" encoding="UTF-8" ?><RESPONSEINFO><FIRMUPDATEINFO><VERSIONCHECK>0</VERSIONCHECK><MEMORYVERSION>a</MEMORYVERSION><FIRMID>FIRM</FIRMID><LATESTVERSION>L1305070834</LATESTVERSION><PATH>http://update-akamai.brother.co.jp/CS/LZ3827_L.djf</PATH><DLTIME>130000</DLTIME></FIRMUPDATEINFO></RESPONSEINFO>

After reading the code, it was clear that it should have worked. I changed the error handling to actually capture and print the exception:

diff --git a/brother_printer_fwupd.py b/brother_printer_fwupd.py
index 6ae68e2..a8e6cc5 100755
--- a/brother_printer_fwupd.py
+++ b/brother_printer_fwupd.py
@@ -179,11 +179,12 @@ def get_download_url(printer_info: PrinterInfo) -> str:
     try:
         resp_xml = BeautifulSoup(resp.text, "xml")
         return resp_xml.find('PATH').text
-    except:
+    except BaseException as e:
         print('[!] Did not receive any url.', file=sys.stderr)
         print('[!] Maybe the firmware is already up to date or there is a bug.', file=sys.stderr)
         print('[!] This is the response of brothers update API:', file=sys.stderr)
         print(resp.text)
+        print(e)
         sys.exit(1)

And now I get this:

[i] Querying printer info via SNMP.
[i]   Detected MFC-J430W with 1 firmware parts.
[i] Querying firmware download URL from brother update API.
[!] Did not receive any url.
[!] Maybe the firmware is already up to date or there is a bug.
[!] This is the response of brothers update API:
<?xml version="1.0" encoding="UTF-8" ?><RESPONSEINFO><FIRMUPDATEINFO><VERSIONCHECK>0</VERSIONCHECK><MEMORYVERSION>a</MEMORYVERSION><FIRMID>FIRM</FIRMID><LATESTVERSION>L1305070834</LATESTVERSION><PATH>http://update-akamai.brother.co.jp/CS/LZ3827_L.djf</PATH><DLTIME>130000</DLTIME></FIRMUPDATEINFO></RESPONSEINFO>
Couldn't find a tree builder with the features you requested: xml. Do you need to install a parser library?

Will figure out myself, but you might want to include this exception handling.

frioux commented 3 years ago

After installing lxml the above issue was resolved, and I was successfully able to update my printer.

sedrubal commented 3 years ago

Thank you for your comment. Yes, we should not except all exceptions. I refactored the code with 643f5afa and added lxml as dependency with 86efdb60. Can you test the program? I don't have access to a brother printer right now...

frioux commented 3 years ago

Seems to still work

On Mon, Dec 07, 2020 at 07:53:27AM -0800, sedrubal wrote:

Thank you for your comment. Yes, we should not except all exceptions. I refactored the code with 643f5afa and added lxml as dependency with 86efdb60. Can you test the program? I don't have access to a brother printer right now...

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/sedrubal/brother_printer_fwupd/issues/1#issuecomment-740005159

-- fREW Schmidt https://blog.afoolishmanifesto.com

sedrubal commented 3 years ago

Thanks 😉