[PATCH 4 of 6] httppeer: wrap HTTPResponse.read() globally

Yuya Nishihara yuya at tcha.org
Sat Apr 15 23:34:05 EDT 2017

On Fri, 14 Apr 2017 00:44:08 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1492155236 25200
> #      Fri Apr 14 00:33:56 2017 -0700
> # Node ID b8a66f70caadbe53bf2ea43b0be1d1d8acba94ad
> # Parent  80bd24abcf67d5dc8b5f5bf83c796e1f71fc5bd9
> httppeer: wrap HTTPResponse.read() globally
> There were a handful of places in the code where HTTPResponse.read()
> was called with no explicit error handling or with inconsistent
> error handling. In order to eliminate this class of bug, we globally
> swap out HTTPResponse.read() with a unified error handler.
> I initially attempted to fix all call sites. However, after
> going down that rabbit hole, I figured it was best to just change
> read() to do what we want. This appears to be a worthwhile
> change, as the tests demonstrate many of our uncaught exceptions
> go away.

After reading the whole series, I agree. Queued but for the last one, thanks.

> To better represent this class of failure, we introduce a new
> error type. The main benefit over IOError is it can hold a hint.
> I'm receptive to tweaking its name or inheritance.

How about WireTransportError? RichIOError seems a bit confusing since it
isn't (and shouldn't be) an IOError.

More information about the Mercurial-devel mailing list