Patchwork traceback: allow disabling the third party extensions blaming

login
register
mail settings
Submitter Pierre-Yves David
Date Sept. 11, 2015, 7:58 p.m.
Message ID <f4cac314b745109687b9.1442001521@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/10487/
State Changes Requested
Headers show

Comments

Pierre-Yves David - Sept. 11, 2015, 7:58 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1442000719 25200
#      Fri Sep 11 12:45:19 2015 -0700
# Node ID f4cac314b745109687b94808f8802b65b39f2dc9
# Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
traceback: allow disabling the third party extensions blaming

The extensions blaming code is fine for casual users but pretty terrible for
corporate environments that use a large amount of extensions. For example, all
reports we get from my users always blame crecord, who is never guilty. This
confused users and triggers endless debug attempts from them.

We add a option to disable this logic, reducing California psychiatric sector
gross worth by a significant amount.
Matt Mackall - Sept. 13, 2015, 12:42 a.m.
On Fri, 2015-09-11 at 12:58 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1442000719 25200
> #      Fri Sep 11 12:45:19 2015 -0700
> # Node ID f4cac314b745109687b94808f8802b65b39f2dc9
> # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
> traceback: allow disabling the third party extensions blaming
> 
> The extensions blaming code is fine for casual users but pretty terrible for
> corporate environments that use a large amount of extensions. For example, all
> reports we get from my users always blame crecord, who is never guilty.

But it *is* guilty. In particular, it is guilty of being unmaintained
and never setting the tested-with field that would take it off the
probably-broken list.

>  This
> confused users and triggers endless debug attempts from them.

You've mistaken this code for a debugging aide. That is not its intent
at all. Instead, it exists to reduce the flow of bug reports for
third-party extensions that waste the project's scarce time.

Ironically, the original primary offender here was Sun developers, who
sent us a steady stream of bizarre reports related to their cadmium
extension. I do not want to see a steady stream of remotefilelog reports
on our BTS from the company that took over their campus.

Instead, write a patch that allows site installs to redirect all bug
reports to their own support contact. That's a feature we can document.
Augie Fackler - Sept. 14, 2015, 2:20 a.m.
On Sat, Sep 12, 2015 at 05:42:37PM -0700, Matt Mackall wrote:
> On Fri, 2015-09-11 at 12:58 -0700, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@fb.com>
> > # Date 1442000719 25200
> > #      Fri Sep 11 12:45:19 2015 -0700
> > # Node ID f4cac314b745109687b94808f8802b65b39f2dc9
> > # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
> > traceback: allow disabling the third party extensions blaming
> >
> > The extensions blaming code is fine for casual users but pretty terrible for
> > corporate environments that use a large amount of extensions. For example, all
> > reports we get from my users always blame crecord, who is never guilty.
>
> But it *is* guilty. In particular, it is guilty of being unmaintained
> and never setting the tested-with field that would take it off the
> probably-broken list.
>
> >  This
> > confused users and triggers endless debug attempts from them.
>
> You've mistaken this code for a debugging aide. That is not its intent
> at all. Instead, it exists to reduce the flow of bug reports for
> third-party extensions that waste the project's scarce time.
>
> Ironically, the original primary offender here was Sun developers, who
> sent us a steady stream of bizarre reports related to their cadmium
> extension. I do not want to see a steady stream of remotefilelog reports
> on our BTS from the company that took over their campus.
>
> Instead, write a patch that allows site installs to redirect all bug
> reports to their own support contact. That's a feature we can document.

I endorse everything mpm said in this mail, and agree that something
like ui.companybuglink would be the thing to do (feel free to change
the color of that bike shed.)

>
> --
> Mathematics is the supreme nostalgia of our time.
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Sept. 14, 2015, 3:53 p.m.
(disclaimer, I'm overall positive with the Matt sugguestion, but have 
some divergence on some details)

On 09/12/2015 05:42 PM, Matt Mackall wrote:
> On Fri, 2015-09-11 at 12:58 -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1442000719 25200
>> #      Fri Sep 11 12:45:19 2015 -0700
>> # Node ID f4cac314b745109687b94808f8802b65b39f2dc9
>> # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
>> traceback: allow disabling the third party extensions blaming
>>
>> The extensions blaming code is fine for casual users but pretty terrible for
>> corporate environments that use a large amount of extensions. For example, all
>> reports we get from my users always blame crecord, who is never guilty.
>
> But it *is* guilty. In particular, it is guilty of being unmaintained
> and never setting the tested-with field that would take it off the
> probably-broken list.

Your definition of guilty is kinda wide. We have a large amount of 
simple and useful third party extensions that are not updated every 
months. I'm not sure it seems realistic to see their lack of update as 
critical (even if getting them tested with latest mercurial is one of 
the goal here).

>
>>   This
>> confused users and triggers endless debug attempts from them.
>
> You've mistaken this code for a debugging aide. That is not its intent
> at all. Instead, it exists to reduce the flow of bug reports for
> third-party extensions that waste the project's scarce time.

The user debugging journey starts with the message.

** Unknown exception encountered with possibly-broken third-party 
extension foobar
** which supports versions 4.2 of Mercurial.
** Please disable foobar and try your action again.
** If that fixes the bug please report it to http://foo.bar/bts

The "please disable foobar and try you action again" part definitely 
lead to users trying to do stuff.

> Ironically, the original primary offender here was Sun developers, who
> sent us a steady stream of bizarre reports related to their cadmium
> extension. I do not want to see a steady stream of remotefilelog reports
> on our BTS from the company that took over their campus.
>
> Instead, write a patch that allows site installs to redirect all bug
> reports to their own support contact. That's a feature we can document.

I like the idea of having a different bugs report flow for big 
deployment that have there own support layer. We still need to do 
something about the user confusion and attempt to debug by disabling a 
long list of extensions one by one.
Sean Farley - Sept. 17, 2015, 9:43 p.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> (disclaimer, I'm overall positive with the Matt sugguestion, but have 
> some divergence on some details)
>
> On 09/12/2015 05:42 PM, Matt Mackall wrote:
>> On Fri, 2015-09-11 at 12:58 -0700, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1442000719 25200
>>> #      Fri Sep 11 12:45:19 2015 -0700
>>> # Node ID f4cac314b745109687b94808f8802b65b39f2dc9
>>> # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
>>> traceback: allow disabling the third party extensions blaming
>>>
>>> The extensions blaming code is fine for casual users but pretty terrible for
>>> corporate environments that use a large amount of extensions. For example, all
>>> reports we get from my users always blame crecord, who is never guilty.
>>
>> But it *is* guilty. In particular, it is guilty of being unmaintained
>> and never setting the tested-with field that would take it off the
>> probably-broken list.
>
> Your definition of guilty is kinda wide. We have a large amount of 
> simple and useful third party extensions that are not updated every 
> months. I'm not sure it seems realistic to see their lack of update as 
> critical (even if getting them tested with latest mercurial is one of 
> the goal here).
>
>>
>>>   This
>>> confused users and triggers endless debug attempts from them.
>>
>> You've mistaken this code for a debugging aide. That is not its intent
>> at all. Instead, it exists to reduce the flow of bug reports for
>> third-party extensions that waste the project's scarce time.
>
> The user debugging journey starts with the message.
>
> ** Unknown exception encountered with possibly-broken third-party 
> extension foobar
> ** which supports versions 4.2 of Mercurial.
> ** Please disable foobar and try your action again.
> ** If that fixes the bug please report it to http://foo.bar/bts
>
> The "please disable foobar and try you action again" part definitely 
> lead to users trying to do stuff.
>
>> Ironically, the original primary offender here was Sun developers, who
>> sent us a steady stream of bizarre reports related to their cadmium
>> extension. I do not want to see a steady stream of remotefilelog reports
>> on our BTS from the company that took over their campus.
>>
>> Instead, write a patch that allows site installs to redirect all bug
>> reports to their own support contact. That's a feature we can document.
>
> I like the idea of having a different bugs report flow for big 
> deployment that have there own support layer. We still need to do 
> something about the user confusion and attempt to debug by disabling a 
> long list of extensions one by one.

I have never once gotten anything useful out of the "possibly-broken"
list because it has never been correct. In fact, I've trained myself to
completely ignore it. I'm positive end-users are baffled as well. I like
Pierre-Yves' direction but don't know how to proceed.
Augie Fackler - Sept. 22, 2015, 1:49 p.m.
On Thu, Sep 17, 2015 at 02:43:56PM -0700, Sean Farley wrote:
>
> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
> >
> > I like the idea of having a different bugs report flow for big
> > deployment that have there own support layer. We still need to do
> > something about the user confusion and attempt to debug by disabling a
> > long list of extensions one by one.
>
> I have never once gotten anything useful out of the "possibly-broken"
> list because it has never been correct. In fact, I've trained myself to
> completely ignore it. I'm positive end-users are baffled as well. I like
> Pierre-Yves' direction but don't know how to proceed.

Perhaps we should list *all* the extensions with outdated or missing
testedwith lines, rather than merely picking one.

I still think pointing to extensions is of value. In general users
with unsupported extensions need to be pushed into testing their bug
without the broken extension before we hear about it.

(This code dramatically reduced noise on the bugtracker from broken
third-party extensions.)

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Sept. 22, 2015, 1:57 p.m.
On Sat, Sep 12, 2015 at 05:42:37PM -0700, Matt Mackall wrote:
> On Fri, 2015-09-11 at 12:58 -0700, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@fb.com>
> > # Date 1442000719 25200
> > #      Fri Sep 11 12:45:19 2015 -0700
> > # Node ID f4cac314b745109687b94808f8802b65b39f2dc9
> > # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
> > traceback: allow disabling the third party extensions blaming
> >
>
> Instead, write a patch that allows site installs to redirect all bug
> reports to their own support contact. That's a feature we can document.

I'd like this feature as described by mpm. Pierre-yves, do you want to
do it or should I?


>
> --
> Mathematics is the supreme nostalgia of our time.
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Sept. 22, 2015, 5:32 p.m.
On 09/22/2015 06:57 AM, Augie Fackler wrote:
> On Sat, Sep 12, 2015 at 05:42:37PM -0700, Matt Mackall wrote:
>> On Fri, 2015-09-11 at 12:58 -0700, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1442000719 25200
>>> #      Fri Sep 11 12:45:19 2015 -0700
>>> # Node ID f4cac314b745109687b94808f8802b65b39f2dc9
>>> # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
>>> traceback: allow disabling the third party extensions blaming
>>>
>>
>> Instead, write a patch that allows site installs to redirect all bug
>> reports to their own support contact. That's a feature we can document.
>
> I'd like this feature as described by mpm. Pierre-yves, do you want to
> do it or should I?

You mean: https://selenic.com/hg/rev/bf2bfc6f45fb ?

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -333,30 +333,31 @@  def _runcatch(req):
         # of date) will be clueful enough to notice the implausible
         # version number and try updating.
         compare = myver.split('+')[0]
         ct = tuplever(compare)
         worst = None, ct, ''
-        for name, mod in extensions.extensions():
-            testedwith = getattr(mod, 'testedwith', '')
-            report = getattr(mod, 'buglink', _('the extension author.'))
-            if not testedwith.strip():
-                # We found an untested extension. It's likely the culprit.
-                worst = name, 'unknown', report
-                break
+        if ui.configbool('ui', 'blamethirdpartyextensions', True):
+            for name, mod in extensions.extensions():
+                testedwith = getattr(mod, 'testedwith', '')
+                report = getattr(mod, 'buglink', _('the extension author.'))
+                if not testedwith.strip():
+                    # We found an untested extension. It's likely the culprit.
+                    worst = name, 'unknown', report
+                    break
 
-            # Never blame on extensions bundled with Mercurial.
-            if testedwith == 'internal':
-                continue
+                # Never blame on extensions bundled with Mercurial.
+                if testedwith == 'internal':
+                    continue
 
-            tested = [tuplever(t) for t in testedwith.split()]
-            if ct in tested:
-                continue
+                tested = [tuplever(t) for t in testedwith.split()]
+                if ct in tested:
+                    continue
 
-            lower = [t for t in tested if t < ct]
-            nearest = max(lower or tested)
-            if worst[0] is None or nearest < worst[1]:
-                worst = name, nearest, report
+                lower = [t for t in tested if t < ct]
+                nearest = max(lower or tested)
+                if worst[0] is None or nearest < worst[1]:
+                    worst = name, nearest, report
         if worst[0] is not None:
             name, testedwith, report = worst
             if not isinstance(testedwith, str):
                 testedwith = '.'.join([str(c) for c in testedwith])
             warning = (_('** Unknown exception encountered with '
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1396,10 +1396,15 @@  User interface controls.
     neither ``$HGUSER`` nor ``$EMAIL`` has been specified, then the user will
     be prompted to enter a username. If no username is entered, the
     default ``USER@HOST`` is used instead.
     (default: False)
 
+``blamethirdpartyextensions``
+    Search for outdated third party extensions involved in crash and point to
+    their bug tracker in the error message.
+    (default: True)
+
 ``commitsubrepos``
     Whether to commit modified subrepositories when committing the
     parent repository. If False and one subrepository has uncommitted
     changes, abort the commit.
     (default: False)
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -942,10 +942,19 @@  Older extension is tested with current v
   ** If that fixes the bug please report it to http://example.com/bts
   ** Python * (glob)
   ** Mercurial Distributed SCM (version 1.9.3)
   ** Extensions loaded: throw, older
 
+Ability to disable that logic
+  $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
+  >   --config ui.blamethirdpartyextensions=false throw 2>&1 | egrep '^\*\*'
+  ** unknown exception encountered, please report by visiting
+  ** http://mercurial.selenic.com/wiki/BugTracker
+  ** Python * (glob)
+  ** Mercurial Distributed SCM (*) (glob)
+  ** Extensions loaded: throw, older
+
 Declare the version as supporting this hg version, show regular bts link:
   $ hgver=`$PYTHON -c 'from mercurial import util; print util.version().split("+")[0]'`
   $ echo 'testedwith = """'"$hgver"'"""' >> throw.py
   $ if [ -z "$hgver" ]; then
   >   echo "unable to fetch a mercurial version. Make sure __version__ is correct";