Opened 6 years ago

Closed 7 months ago

#5310 defect closed fixed (fixed)

WrapperException makes debugging Agent hard

Reported by: Glyph Owned by: Adi Roiban
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight, twm@… Branch: agent-exceptions-5310
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

If request generation fails with Agent it is difficult to debug because _WrapperException swallows tracebacks automatically and provides no facility to extract them manually.

For example, this (buggy) program:

from twisted.internet import reactor
from twisted.web.client import Agent
agent = Agent(reactor)
class Whatever(object):
    """
    I should probably implement some interfaces or something.
    """
def done(it):
    reactor.stop()
    return it
(agent.request("POST", "http://example.com/", bodyProducer=Whatever())
      .addBoth(done))
reactor.run()

produces the following inscrutable output:

Unhandled error in Deferred:
Unhandled Error
Traceback (most recent call last):
Failure: twisted.web._newclient.RequestGenerationFailed: [
    <twisted.python.failure.Failure <type 'exceptions.AttributeError'>>]

RequestGenerationFailed apparently has no public alias; neither does _WrapperException, which means that there's no way to catch this exception and do better error reporting without resorting to importing private names.

Also, _WrapperException's documentation is wrong: while its subclasses get things right, it says that reasons is a list of exceptions, when it is in fact a list of Failures.

Change History (11)

comment:1 Changed 6 years ago by DefaultCC Plugin

Cc: jknight added

comment:2 Changed 6 years ago by Jean-Paul Calderone

For some reason this was filed again as #5345

comment:3 Changed 5 years ago by therve

And was already filed as #4659, but I kept this one.

comment:4 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/agent-exceptions-5310

(In [41819]) Branching to 'agent-exceptions-5310'

comment:5 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added

comment:6 Changed 4 years ago by Glyph

Keywords: review removed
Owner: set to Jean-Paul Calderone

Thanks for addressing this, exarkun!

I think it's OK to land as you see fit. Thanks for the import cleanups.

The one thought that occurs to me, looking at this, is that most users will encounter this exception initially via its repr, and will have no particular context for discerning its public alias. This is, of course, a more general issue, but if you would like to add a __repr__ and a test for it that adjusts the name so that users will find the right API documentation to look at, that would be a good start.

Perhaps this also needs some narrative documentation that explains how you might hit this sort of error though.

Also, do you think we should file a separate ticket for somehow making printTraceback print the wrapped exception's traceback too? Part of the inscrutability of the error here is that, by default, when it's reported, it's really just not clear at all why, and the type of the wrapped exception(s) isn't enough information to discern that.

comment:7 Changed 22 months ago by Tom Most

Cc: twm@… added

comment:8 Changed 8 months ago by Adi Roiban

Branch: branches/agent-exceptions-5310agent-exceptions-5310

comment:9 Changed 8 months ago by Adi Roiban

Keywords: review added
Owner: Jean-Paul Calderone deleted

Not sure why this was not merged yet.

I have created a new branch based on the original branch.

PR at https://github.com/twisted/twisted/pull/754

Thanks!

comment:10 Changed 7 months ago by Glyph

Keywords: review removed
Owner: set to Adi Roiban

OK, this looks good to me. Please merge once the builds complete and (hopefully) succeed.

comment:11 Changed 7 months ago by Adi Roiban <adi.roiban@…>

Resolution: fixed
Status: newclosed

In 98049a3:

Merge pull request #754 from twisted/5310-agent-exceptions-take2

Author: adiroiban, exarkun
Reviewer: glyph
Fixes: ticket:5310

Make t.web.client.RequestGenerationFailed a public API.

Note: See TracTickets for help on using tickets.