Page MenuHomePhabricator

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

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

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Thu, Nov 12, 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.Fri, Nov 13, 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.Mon, Nov 16, 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.Mon, Nov 16, 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.Tue, Nov 17, 12:55 AM
This revision is now accepted and ready to land.Tue, Nov 17, 12:55 AM