Submitter | Augie Fackler |
---|---|
Date | Sept. 15, 2017, 11:14 p.m. |
Message ID | <662bbd6d96952985eff8.1505517244@augie-macbookpro2.roam.corp.google.com> |
Download | mbox | patch |
Permalink | /patch/23924/ |
State | Accepted |
Headers | show |
Comments
On Fri, 15 Sep 2017 19:14:04 -0400, Augie Fackler wrote: > # HG changeset patch > # User Augie Fackler <raf@durin42.com> > # Date 1505515049 14400 > # Fri Sep 15 18:37:29 2017 -0400 > # Node ID 662bbd6d96952985eff807f424dd128663724672 > # Parent 209120041d12b524648fa856732aa404dfedd91d > bundle2: raise a more helpful error if building a bundle part header fails > > I've tripped on this several times now, and am tired of debugging. Now > the header parts are part of the error message when the ''.join() > fails, which makes debugging obvious. > > diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py > --- a/mercurial/bundle2.py > +++ b/mercurial/bundle2.py > @@ -1050,7 +1050,11 @@ class bundlepart(object): > header.append(key) > header.append(value) > ## finalize header > - headerchunk = ''.join(header) > + try: > + headerchunk = ''.join(header) > + except TypeError: > + raise TypeError(u'Found a non-bytes trying to ' > + u'build bundle part header: %r' % header) I was making it a r'' string, but probably ProgrammingError would be better.
> On Sep 16, 2017, at 7:54 AM, Yuya Nishihara <yuya@tcha.org> wrote: > > On Fri, 15 Sep 2017 19:14:04 -0400, Augie Fackler wrote: >> # HG changeset patch >> # User Augie Fackler <raf@durin42.com> >> # Date 1505515049 14400 >> # Fri Sep 15 18:37:29 2017 -0400 >> # Node ID 662bbd6d96952985eff807f424dd128663724672 >> # Parent 209120041d12b524648fa856732aa404dfedd91d >> bundle2: raise a more helpful error if building a bundle part header fails >> >> I've tripped on this several times now, and am tired of debugging. Now >> the header parts are part of the error message when the ''.join() >> fails, which makes debugging obvious. >> >> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py >> --- a/mercurial/bundle2.py >> +++ b/mercurial/bundle2.py >> @@ -1050,7 +1050,11 @@ class bundlepart(object): >> header.append(key) >> header.append(value) >> ## finalize header >> - headerchunk = ''.join(header) >> + try: >> + headerchunk = ''.join(header) >> + except TypeError: >> + raise TypeError(u'Found a non-bytes trying to ' >> + u'build bundle part header: %r' % header) > > I was making it a r'' string, but probably ProgrammingError would be better. I was conflicted on this one. Preserving the type of the exception seemed reasonable, since this can only happen with weird bugs, but it’s also something I can just sit on for a while as a debugging aid.
On Sat, 16 Sep 2017 11:08:07 -0400, Augie Fackler wrote: > > > On Sep 16, 2017, at 7:54 AM, Yuya Nishihara <yuya@tcha.org> wrote: > > > > On Fri, 15 Sep 2017 19:14:04 -0400, Augie Fackler wrote: > >> # HG changeset patch > >> # User Augie Fackler <raf@durin42.com> > >> # Date 1505515049 14400 > >> # Fri Sep 15 18:37:29 2017 -0400 > >> # Node ID 662bbd6d96952985eff807f424dd128663724672 > >> # Parent 209120041d12b524648fa856732aa404dfedd91d > >> bundle2: raise a more helpful error if building a bundle part header fails > >> > >> I've tripped on this several times now, and am tired of debugging. Now > >> the header parts are part of the error message when the ''.join() > >> fails, which makes debugging obvious. > >> > >> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py > >> --- a/mercurial/bundle2.py > >> +++ b/mercurial/bundle2.py > >> @@ -1050,7 +1050,11 @@ class bundlepart(object): > >> header.append(key) > >> header.append(value) > >> ## finalize header > >> - headerchunk = ''.join(header) > >> + try: > >> + headerchunk = ''.join(header) > >> + except TypeError: > >> + raise TypeError(u'Found a non-bytes trying to ' > >> + u'build bundle part header: %r' % header) > > > > I was making it a r'' string, but probably ProgrammingError would be better. > > I was conflicted on this one. Preserving the type of the exception seemed reasonable, since this can only happen with weird bugs, but it’s also something I can just sit on for a while as a debugging aid. Okay, queued with s/u'/r'/ change in flight, thanks.
Patch
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py --- a/mercurial/bundle2.py +++ b/mercurial/bundle2.py @@ -1050,7 +1050,11 @@ class bundlepart(object): header.append(key) header.append(value) ## finalize header - headerchunk = ''.join(header) + try: + headerchunk = ''.join(header) + except TypeError: + raise TypeError(u'Found a non-bytes trying to ' + u'build bundle part header: %r' % header) outdebug(ui, 'header chunk size: %i' % len(headerchunk)) yield _pack(_fpartheadersize, len(headerchunk)) yield headerchunk