RPM Importer swallows exception when one is raised during upload
Originally written by email@example.com, recreated after I accidentally deleted the issue
When uploading an invalid rpm, you get a generic error message: "The importer yum_importer indicated a failed response when uploading rpm unit to repository zoolander." I'd expect some sort of error message about the rpm being invalid instead. Even something like the message logged in journalctl would suffice (ie "error reading package header").
Steps to reproduce:
touch meerkat-2.1.0.noarch.rpm pulp-admin rpm repo uploads rpm -f meerkat-2.1.0.noarch.rpm --repo-id zoolander
Updated by dalley over 5 years ago
When I test this, I do get such an error message (it's logged before the traceback instead of after, so it is not as prominent and easy to miss).
Jan 24 21:37:01 dev.example.com pulp: pulp_rpm.plugins.importers.yum.upload:ERROR: [a087e7b5] (4444-63232) Error extracting RPM metadata for [/var/lib/pulp/uploads/30820126-11c3-4dfe-a585-67b7fa68cbeb]
I do think this message is still overly vague and technical, and could be improved a lot. I think we could also pass the information about the cause of the failure from where it is first logged and include it in the later error message that you've pointed out in the description.
Updated by daviddavis over 5 years ago
dalley that works. As long as the pulp-admin user sees a specific error messages besides "The importer yum_importer indicated a failed response..." without having to look in the log files.
Also, another thing to consider: we'll want to make sure that the actual error is being passed back in the api (this might already be the case) so Katello for instance can pass this on to their users.
Updated by bmbouter over 5 years ago
+1 to having a good error message be shown to the user via the API and pulp-admin. I believe this is as simple as raising an exception. The message of the exception will be stored on the TaskStatus. I think that happens here. These "exception swalling" issues I think are mainly fatal errors that should raise an Exception but don't.
The main issue with "fixing" these issues is that there could be important code that would not get run and that could cause behavioral backwards incompatabilities. So that is something to consider. @dkliban and mhrivnak are also well versed in these things topics.
Note that I think this is about the exception being unraised or a fatal exception being treated as a non-fatal exception which causes pulp-admin to not receive the errors and instead it shows success. The error message and logging could be improved, but that would be an additional improvement on top of the primary goal. I could be wrong about this interpretation from the original report, but I wanted to post some thoughts.