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

Minor improvements and extra logging for DesktopNetworkListener #1561

Merged
merged 2 commits into from
May 14, 2023

Conversation

amoerie
Copy link
Collaborator

@amoerie amoerie commented May 11, 2023

If a generic exception occurs while trying to listen for inbound connections, at the very least we should log that.
Secondly, exceptions that occur there would be wrapped inside an AggregateException because we used .Result.
Lastly, this PR also adds a little bit of debug logging.

Checklist

  • The pull request branch is in sync with latest commit on the fo-dicom/development branch
  • I have updated API documentation
  • I have included unit tests
  • I have updated the change log
  • I am listed in the CONTRIBUTORS file

Changes proposed in this pull request:

  • Add logging to DesktopNetworkListener
  • Use referential equality to check for the Task.WhenAny winner
  • Use await instead of .Result so exceptions are not wrapped in an unnecessary AggregateException

@amoerie amoerie changed the title Add logging to DesktopNetworkListener Minor improvements and extra logging forDesktopNetworkListener May 11, 2023
@amoerie amoerie changed the title Minor improvements and extra logging forDesktopNetworkListener Minor improvements and extra logging for DesktopNetworkListener May 11, 2023
@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (1d9c03c) 79.80% compared to head (7009a2b) 79.80%.

Additional details and impacted files
@@             Coverage Diff              @@
##           development    #1561   +/-   ##
============================================
  Coverage        79.80%   79.80%           
============================================
  Files              263      263           
  Lines            24659    24660    +1     
============================================
+ Hits             19678    19679    +1     
  Misses            4981     4981           
Impacted Files Coverage Δ
FO-DICOM.Core/Network/DicomServer.cs 92.77% <ø> (ø)
FO-DICOM.Core/Network/DesktopNetworkListener.cs 46.66% <100.00%> (+3.80%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

If a generic exception occurs while trying to listen for inbound connections, at the very least we should log that
@mrbean-bremen mrbean-bremen merged commit e441a1a into development May 14, 2023
6 checks passed
@gofal gofal deleted the listener-logging branch May 14, 2023 19:03
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.

None yet

3 participants