Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PICARD-2740: Improve logging of SSL errors #2294

Merged
merged 1 commit into from
Aug 30, 2023
Merged

Conversation

zas
Copy link
Collaborator

@zas zas commented Aug 29, 2023

Summary

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences:

Problem

Solution

Action

@zas zas requested a review from phw August 29, 2023 09:51
Copy link
Member

@phw phw left a comment

Choose a reason for hiding this comment

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

That's a bit difficult to test in real. But wouldn't this lead to double logging, as we already log all errors in https://github.com/metabrainz/picard/blob/master/picard/webservice/__init__.py#L511 as well?

@zas
Copy link
Collaborator Author

zas commented Aug 29, 2023

That's a bit difficult to test in real. But wouldn't this lead to double logging, as we already log all errors in https://github.com/metabrainz/picard/blob/master/picard/webservice/__init__.py#L511 as well?

You can use the commented out code to trigger an error, but basically it provides more details than the existing message;

E: 12:25:28,378 webservice.ssl_errors:363: SSL error: The issuer certificate of a locally looked up certificate could not be found
E: 12:25:28,378 webservice._handle_reply:534: Network request error for https://picard.musicbrainz.org/api/v2/plugins/ -> SSL handshake failed (QT code 6, HTTP code 0)
E: 12:25:28,379 ui/mainwindow.set_statusbar_message:469: Error loading plugins list: SSL handshake failed

The existing message only says the SSL handshake failed, but the new message actually says why.

@zas zas changed the title Log SSL errors PICARD-2740: Improve logging of SSL errors Aug 30, 2023
@zas zas merged commit 11c6045 into metabrainz:master Aug 30, 2023
63 checks passed
@zas zas deleted the sslerrors branch August 30, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants