This is an archive of the discontinued Mercurial Phabricator instance.

bundle2: immediate exit for ctrl+c (issue5692)
ClosedPublic

Authored by durham on Oct 5 2017, 5:20 PM.

Details

Summary

21c2df59a regressed bundle2 by catching all exceptions and trying to handle
them. The old behavior was to allow KeyboardInterrupts to throw and not have
graceful cleanup, which allowed it to exit immediately. Let's go back to that
behavior.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durham created this revision.Oct 5 2017, 5:20 PM
quark accepted this revision.Oct 5 2017, 6:31 PM
quark requested changes to this revision.
quark added inline comments.
mercurial/bundle2.py
380

Actually KeyboardInterrupt is tested here. So there is something else going on. Maybe we should add error.SignalInterrupt to the list?

This revision now requires changes to proceed.Oct 5 2017, 6:32 PM
yuja added a subscriber: yuja.Oct 6 2017, 11:05 AM
yuja added inline comments.
mercurial/bundle2.py
380

SignalInterrupt is a subclass of KeyboardInterrupt, so there might
be deeper problem.

FWIW, I think it's probably a good idea to replace this blacklist
with isinstance(exc, Exception).

durham added inline comments.Oct 11 2017, 1:33 PM
mercurial/bundle2.py
380

Before my refactor there used to be two levels of error handling, 1) inside _parthandler which tried to seek to the end if it wasn't systemexit or keyboardinterrup, and 2) inside processbundle (which calls _parthandler) which only caught type Exception.

It seems like the correct unification of these would be to just keep the isinstance(exc, Exception) check above, and delete this sub-check entirely. I'll send an update.

durham updated this revision to Diff 2592.Oct 11 2017, 1:44 PM
durham marked 3 inline comments as done.Oct 11 2017, 1:44 PM
quark accepted this revision.Oct 11 2017, 5:36 PM
This revision was automatically updated to reflect the committed changes.