[tahoe-lafs-trac-stream] [tahoe-lafs] #1589: S3 backend: when a txaws operation gets an error response, include the full request URI and response body in the exception message, and maybe trigger an incident

[tahoe-lafs]

#1589: S3 backend: when a txaws operation gets an error response, include the full
request URI and response body in the exception message, and maybe trigger
an incident
-----------------------------+---------------------------------------------
     Reporter:  davidsarah   |      Owner:  davidsarah
         Type:  defect       |     Status:  new
     Priority:  major        |  Milestone:  1.10.0
    Component:  code-        |    Version:  1.9.0b1
  storage                    |   Keywords:  error s3-backend txaws reviewed
   Resolution:               |
Launchpad Bug:               |
-----------------------------+---------------------------------------------

Comment (by davidsarah):

 Replying to [comment:6 zooko]:
 > patch review:
 >
 > I guess it is redundant but not harmful to say that class {{{S3Bucket}}}
 {{{implements(IS3Bucket)}}}, since it now subclasses {{{S3BucketMixin}}}
 which {{{implements(IS3Bucket)}}}.

 {{{S3BucketMixin}}} doesn't actually implement {{{S3Bucket}}} (and
 shouldn't, since it only implements one helper method).

 > Why do we need both the {{{f.trap(self.S3Error)}}} and the {{{except
 self.S3Error}}}?

 The intention is to preserve the exception traceback. The {{{trap}}} call
 does so when the exception is not a {{{self.S3Error}}} (because it's
 preserving the same {{{Failure}}}), and the {{{except}}} clause does so
 (using the 3-arg form of {{{raise}}}) when it is. The {{{except}}} clause
 depends on the exception object being an {{{S3Error}}} or
 {{{MockS3Error}}}. I don't know of any other way to preserve the traceback
 than to raise and catch {{{f.value}}}.

 (I attempted to just change the behaviour of stringification by setting
 {{{f.value.__str__}}} rather than using a different exception class, but
 was stymied by what I consider to be a bug in CPython -- {{{str(x)}}} is
 equivalent to {{{x.__class__.__str__(x)}}} and not to {{{x.__str__()}}} as
 the documentation implies, so setting {{{__str__}}} on an instance does
 not work.)

 The other alternative would be to change {{{AWSError.__str__}}} in txaws.
 That might be more elegant, but I would prefer to minimize the changes in
 our txaws fork.

-- 
Ticket URL: <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1589#comment:8>
tahoe-lafs <https://tahoe-lafs.org>
secure decentralized storage