Skip to content

[upload] Fix backtrace when cancelling upload#4057

Merged
arif-ali merged 1 commit into
sosreport:mainfrom
jcastill:jcastillo-sos-capture-user-exit-upload
Jun 18, 2025
Merged

[upload] Fix backtrace when cancelling upload#4057
arif-ali merged 1 commit into
sosreport:mainfrom
jcastill:jcastillo-sos-capture-user-exit-upload

Conversation

@jcastill

Copy link
Copy Markdown
Member

This commit fix a backtrace that appears when
the user cancels a file upload after authentication.

Related: RHEL-94968


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

This commit fix a backtrace that appears when
the user cancels a file upload after authentication.

Related: RHEL-94968

Signed-off-by: Jose Castillo <jcastillo@redhat.com>
@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-4057
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Comment thread sos/upload/__init__.py
self.ui_log.error(_(f"Cannot upload {self.archive}: {e} "))
except KeyboardInterrupt:
self._exit("Exiting on user cancel", 130)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about adding a general exception after KeyboardInterrupt to capture any other uncatched exceptions. Any thoughts on this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might as well, of there is a chance for another issue to happen?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to come up with possible ways to trigger other exceptions, but haven't found a way to get one yet. Rather, I thought I could add them generally after being explicit with the KeyboardInterrupt one, and if we capture one we can refine it later on (i.e. add a new explicit one before the general one).

@jcastill jcastill requested a review from pmoravec June 17, 2025 12:51
@pmoravec pmoravec added Reviewed/Needs 2nd Ack Require a 2nd ack from a maintainer Kind/Upload upload component of sos labels Jun 17, 2025
@jcastill

Copy link
Copy Markdown
Member Author

@pmoravec @arif-ali shall I add a general exception catch then and force push? Or do you prefer if I leave it as it's now?

@arif-ali arif-ali left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you've tried other means, lets stick with what we have, and we move on

@arif-ali arif-ali added Reviewed/Ready for Merge Has been reviewed, ready for merge and removed Reviewed/Needs 2nd Ack Require a 2nd ack from a maintainer labels Jun 18, 2025
@arif-ali arif-ali merged commit df7fa5b into sosreport:main Jun 18, 2025
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Kind/Upload upload component of sos Reviewed/Ready for Merge Has been reviewed, ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants