This is an archive of the discontinued Mercurial Phabricator instance.

errors: set detailed exit code to 100 for some remote errors
ClosedPublic

Authored by martinvonz on Nov 12 2020, 11:37 AM.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Nov 12 2020, 11:37 AM
mharbison72 added inline comments.
tests/test-clone.t
621

I'm surprised this is a considered a remote error. Maybe it's related to the sniffing for HttpError or URLError in the exception handler? (I'm fine with this if you are- maybe there's no way to distinguish from a remote error).

tests/test-extdata.t
116

Probably the same issue as above with thinking OSError is a remote error?

martinvonz added inline comments.Nov 13 2020, 2:59 AM
tests/test-clone.t
621

I agree. I spent a bit of time trying to convert the errors at a lower level (in keepalive.py) but it seems hard to tell them apart even there. So I'd say it's probably not worth it.

I insert D9318 before this patch to clean up a little.

mharbison72 added inline comments.Nov 16 2020, 1:06 AM
tests/test-extdata.t
116

With the cleanup in D9318, I'm baffled why this change didn't go away. Any idea why it's doing this, or how widespread the misdiagnosis is? (I'm assuming it's got something specific to do with this revset, because I didn't notice anything else that looked wrong in the tests)

martinvonz added inline comments.Nov 16 2020, 1:18 PM
tests/test-extdata.t
116

We still get a URLError here because the path is not found. In that other case in test-clone.t where the user gave us a bad URL, I agree that it would have been better to consider that an InputError (exit code 10). It's less clear-cut where because the user gave us a valid URL but it didn't "respond". Of course, in this case it was a file system path, so it's easier to tell the difference between a URL that doesn't exist and some network error...

mharbison72 accepted this revision.Nov 17 2020, 12:55 AM
This revision is now accepted and ready to land.Nov 17 2020, 12:55 AM