Handle graceful shutdown on TransportUseClosedError.

[AlejandroCabeza]

fixes: #1007

[codecov]

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 82.53%. Comparing base (ca01ee0) to head (3e56745).
Report is 2 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1065      +/-   ##
============================================
- Coverage     82.81%   82.53%   -0.28%     
============================================
  Files            91       91              
  Lines         15750    15822      +72     
============================================
+ Hits          13043    13059      +16     
- Misses         2707     2763      +56     

Files	Coverage Δ
libp2p/switch.nim	88.61% <22.22%> (-3.14%)	⬇️

... and 7 files with indirect coverage changes

[lchenut]

LGTM

[diegomrsantos]

LGTM, just a minor request. It'd be great if we could add a test too.

[AlejandroCabeza]

LGTM, just a minor request. It'd be great if we could add a test too.

That's a good idea, but it might be a tad too hard for me:
Can you point me to an already existing test I can use as template where accept is being tested/called? Also, I don't know how to raise the TransportUseClosedError which is necessary for the test, or how to mock it, for that matter.

Otherwise, this might be too time-consuming (for me).

[diegomrsantos]

we can use the same as below "Exception in accept loop, exiting", exc = exc.msg

[AlejandroCabeza]

Don't you think that might trigger the same reactions, from users, that @cheatfate was mentioning in the issue?

[diegomrsantos]

I think the issue was about the error log level. We can keep the one you used, my comment was about the rest.

[AlejandroCabeza]

It seems to me that it makes sense for the others to still spit error, as long as it's an actual error. The issue, as I understood, is it was logging it as an error something that wasn't an actual error, hence confusing people.

[diegomrsantos]

Users aren't supposed to use trace unless they are trying to debug an issue. That was precisely my intention suggesting "Exception in accept loop, exiting", exc = exc.msg, adding more useful info to the log, especially the exception msg.

[AlejandroCabeza]

@cheatfate

[diegomrsantos]

The issue might be fixed by #1095 (comment). I'm tempted to close this PR.

[diegomrsantos]

@kaiserd

[kaiserd]

Can we test if it actually has been fixed? If yes, let's close this PR and link 1095 in the close message.

[diegomrsantos]

At the moment, I don't know how to reproduce it, if it hasn't been fixed yet.

[diegomrsantos]

If testing it is too time consuming, or not even possible, that's fine.

[kaiserd]

What is the status here?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 84.53%. Comparing base (02c96fc) to head (453bcc6).
Report is 30 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1065      +/-   ##
==========================================
- Coverage   84.53%   84.53%   -0.01%     
==========================================
  Files          91       92       +1     
  Lines       15517    16414     +897     
==========================================
+ Hits        13118    13875     +757     
- Misses       2399     2539     +140     

Files	Coverage Δ
libp2p/switch.nim	89.28% <22.22%> (-2.43%)	⬇️

... and 77 files with indirect coverage changes