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

Gregory Szorc gregory.szorc at gmail.com
Sun Apr 16 14:07:14 EDT 2017


On Sat, Apr 15, 2017 at 8:34 PM, Yuya Nishihara <yuya at tcha.org> wrote:

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

Yeah, I think renaming it to "WireTransportError" or similar is a good
idea. I'll try to send that patch later today...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170416/9f5730fd/attachment.html>


More information about the Mercurial-devel mailing list