Page MenuHomePhabricator

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

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

Diff Detail

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

Event Timeline

martinvonz created this revision.Nov 12 2020, 11:37 AM
mharbison72 added inline comments.

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).


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

martinvonz added inline comments.Nov 13 2020, 2:59 AM

I agree. I spent a bit of time trying to convert the errors at a lower level (in 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

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

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