From 2d34b6a998f3f5eb454e915415e94e1387a1ee06 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 26 Apr 2022 16:29:00 -0400
Subject: [PATCH 01/13] Log the request and response in the server.

---
 src/allmydata/storage/http_server.py | 36 ++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py
index 7c4860d57..935390d10 100644
--- a/src/allmydata/storage/http_server.py
+++ b/src/allmydata/storage/http_server.py
@@ -8,6 +8,7 @@ from functools import wraps
 from base64 import b64decode
 import binascii
 
+from eliot import start_action
 from zope.interface import implementer
 from klein import Klein
 from twisted.web import http
@@ -83,8 +84,9 @@ def _extract_secrets(
 
 def _authorization_decorator(required_secrets):
     """
-    Check the ``Authorization`` header, and extract ``X-Tahoe-Authorization``
-    headers and pass them in.
+    1. Check the ``Authorization`` header matches server swissnum.
+    2. Extract ``X-Tahoe-Authorization`` headers and pass them in.
+    3. Log the request and response.
     """
 
     def decorator(f):
@@ -106,7 +108,22 @@ def _authorization_decorator(required_secrets):
             except ClientSecretsException:
                 request.setResponseCode(http.BAD_REQUEST)
                 return b"Missing required secrets"
-            return f(self, request, secrets, *args, **kwargs)
+            with start_action(
+                action_type="allmydata:storage:http-server:request",
+                method=request.method,
+                path=request.path,
+            ) as ctx:
+                try:
+                    result = f(self, request, secrets, *args, **kwargs)
+                except _HTTPError as e:
+                    # This isn't an error necessarily for logging purposes,
+                    # it's an implementation detail, an easier way to set
+                    # response codes.
+                    ctx.add_success_fields(response_code=e.code)
+                    ctx.finish()
+                    raise
+                ctx.add_success_fields(response_code=request.code)
+                return result
 
         return route
 
@@ -239,19 +256,24 @@ class _HTTPError(Exception):
 # https://www.iana.org/assignments/cbor-tags/cbor-tags.xhtml. Notably, #6.258
 # indicates a set.
 _SCHEMAS = {
-    "allocate_buckets": Schema("""
+    "allocate_buckets": Schema(
+        """
     message = {
       share-numbers: #6.258([* uint])
       allocated-size: uint
     }
-    """),
-    "advise_corrupt_share": Schema("""
+    """
+    ),
+    "advise_corrupt_share": Schema(
+        """
     message = {
       reason: tstr
     }
-    """)
+    """
+    ),
 }
 
+
 class HTTPServer(object):
     """
     A HTTP interface to the storage server.

From 2722e81d2b4c8bffeebbab3be9654f1cb40736ad Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 26 Apr 2022 16:33:48 -0400
Subject: [PATCH 02/13] News file.

---
 newsfragments/3880.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3880.minor

diff --git a/newsfragments/3880.minor b/newsfragments/3880.minor
new file mode 100644
index 000000000..e69de29bb

From bd631665f4c5d66d16574dbab036c76529a16bca Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 4 May 2022 10:21:16 -0400
Subject: [PATCH 03/13] Add logging of HTTP requests from client.

---
 src/allmydata/storage/http_client.py | 88 +++++++++++++++-------------
 1 file changed, 48 insertions(+), 40 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index da350e0c6..b4bde6a95 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -4,6 +4,7 @@ HTTP client that talks to the HTTP storage server.
 
 from __future__ import annotations
 
+from eliot import start_action, register_exception_extractor
 from typing import Union, Optional, Sequence, Mapping
 from base64 import b64encode
 
@@ -55,6 +56,8 @@ class ClientException(Exception):
         Exception.__init__(self, code, *additional_args)
         self.code = code
 
+register_exception_extractor(ClientException, lambda e: {"response_code": e.code})
+
 
 # Schemas for server responses.
 #
@@ -280,6 +283,7 @@ class StorageClient(object):
         )
         return headers
 
+    @inlineCallbacks
     def request(
         self,
         method,
@@ -299,37 +303,40 @@ class StorageClient(object):
         If ``message_to_serialize`` is set, it will be serialized (by default
         with CBOR) and set as the request body.
         """
-        headers = self._get_headers(headers)
+        with start_action(action_type="allmydata:storage:http-client:request", method=method, url=str(url)) as ctx:
+            headers = self._get_headers(headers)
 
-        # Add secrets:
-        for secret, value in [
-            (Secrets.LEASE_RENEW, lease_renew_secret),
-            (Secrets.LEASE_CANCEL, lease_cancel_secret),
-            (Secrets.UPLOAD, upload_secret),
-            (Secrets.WRITE_ENABLER, write_enabler_secret),
-        ]:
-            if value is None:
-                continue
-            headers.addRawHeader(
-                "X-Tahoe-Authorization",
-                b"%s %s" % (secret.value.encode("ascii"), b64encode(value).strip()),
-            )
-
-        # Note we can accept CBOR:
-        headers.addRawHeader("Accept", CBOR_MIME_TYPE)
-
-        # If there's a request message, serialize it and set the Content-Type
-        # header:
-        if message_to_serialize is not None:
-            if "data" in kwargs:
-                raise TypeError(
-                    "Can't use both `message_to_serialize` and `data` "
-                    "as keyword arguments at the same time"
+            # Add secrets:
+            for secret, value in [
+                (Secrets.LEASE_RENEW, lease_renew_secret),
+                (Secrets.LEASE_CANCEL, lease_cancel_secret),
+                (Secrets.UPLOAD, upload_secret),
+                (Secrets.WRITE_ENABLER, write_enabler_secret),
+            ]:
+                if value is None:
+                    continue
+                headers.addRawHeader(
+                    "X-Tahoe-Authorization",
+                    b"%s %s" % (secret.value.encode("ascii"), b64encode(value).strip()),
                 )
-            kwargs["data"] = dumps(message_to_serialize)
-            headers.addRawHeader("Content-Type", CBOR_MIME_TYPE)
 
-        return self._treq.request(method, url, headers=headers, **kwargs)
+            # Note we can accept CBOR:
+            headers.addRawHeader("Accept", CBOR_MIME_TYPE)
+
+            # If there's a request message, serialize it and set the Content-Type
+            # header:
+            if message_to_serialize is not None:
+                if "data" in kwargs:
+                    raise TypeError(
+                        "Can't use both `message_to_serialize` and `data` "
+                        "as keyword arguments at the same time"
+                    )
+                kwargs["data"] = dumps(message_to_serialize)
+                headers.addRawHeader("Content-Type", CBOR_MIME_TYPE)
+
+            response = yield self._treq.request(method, url, headers=headers, **kwargs)
+            ctx.add_success_fields(response_code=response.code)
+            return response
 
 
 class StorageClientGeneral(object):
@@ -538,18 +545,19 @@ class StorageClientImmutables(object):
         """
         Return the set of shares for a given storage index.
         """
-        url = self._client.relative_url(
-            "/v1/immutable/{}/shares".format(_encode_si(storage_index))
-        )
-        response = yield self._client.request(
-            "GET",
-            url,
-        )
-        if response.code == http.OK:
-            body = yield _decode_cbor(response, _SCHEMAS["list_shares"])
-            returnValue(set(body))
-        else:
-            raise ClientException(response.code)
+        with start_action(action_type="allmydata:storage:http-client:immutable:list-shares", storage_index=storage_index) as ctx:
+            url = self._client.relative_url(
+                "/v1/immutable/{}/shares".format(_encode_si(storage_index))
+            )
+            response = yield self._client.request(
+                "GET",
+                url,
+            )
+            if response.code == http.OK:
+                body = yield _decode_cbor(response, _SCHEMAS["list_shares"])
+                return set(body)
+            else:
+                raise ClientException(response.code)
 
     @inlineCallbacks
     def add_or_renew_lease(

From 4211fd8525bfe5e45c323a9389df1ec5f54aab8d Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 27 Mar 2023 13:41:30 -0400
Subject: [PATCH 04/13] Revert to old code.

---
 src/allmydata/storage/http_client.py | 46 ++++++++++++++++------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index a61f94708..44ba64363 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -355,7 +355,6 @@ class StorageClient(object):
         )
         return headers
 
-    @inlineCallbacks
     def request(
         self,
         method,
@@ -378,26 +377,35 @@ class StorageClient(object):
 
         Default timeout is 60 seconds.
         """
-        with start_action(
-            action_type="allmydata:storage:http-client:request",
-            method=method,
-            url=str(url),
-        ) as ctx:
-            headers = self._get_headers(headers)
+        headers = self._get_headers(headers)
 
-            # Add secrets:
-            for secret, value in [
-                (Secrets.LEASE_RENEW, lease_renew_secret),
-                (Secrets.LEASE_CANCEL, lease_cancel_secret),
-                (Secrets.UPLOAD, upload_secret),
-                (Secrets.WRITE_ENABLER, write_enabler_secret),
-            ]:
-                if value is None:
-                    continue
-                headers.addRawHeader(
-                    "X-Tahoe-Authorization",
-                    b"%s %s" % (secret.value.encode("ascii"), b64encode(value).strip()),
+        # Add secrets:
+        for secret, value in [
+            (Secrets.LEASE_RENEW, lease_renew_secret),
+            (Secrets.LEASE_CANCEL, lease_cancel_secret),
+            (Secrets.UPLOAD, upload_secret),
+            (Secrets.WRITE_ENABLER, write_enabler_secret),
+        ]:
+            if value is None:
+                continue
+            headers.addRawHeader(
+                "X-Tahoe-Authorization",
+                b"%s %s" % (secret.value.encode("ascii"), b64encode(value).strip()),
+            )
+
+        # Note we can accept CBOR:
+        headers.addRawHeader("Accept", CBOR_MIME_TYPE)
+
+        # If there's a request message, serialize it and set the Content-Type
+        # header:
+        if message_to_serialize is not None:
+            if "data" in kwargs:
+                raise TypeError(
+                    "Can't use both `message_to_serialize` and `data` "
+                    "as keyword arguments at the same time"
                 )
+            kwargs["data"] = dumps(message_to_serialize)
+            headers.addRawHeader("Content-Type", CBOR_MIME_TYPE)
 
         return self._treq.request(
             method, url, headers=headers, timeout=timeout, **kwargs

From e8c72e6753db8287ef1dcfa824080e1191746c2a Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Tue, 28 Mar 2023 12:55:41 -0400
Subject: [PATCH 05/13] Not sure if per method logging is worth it, will start
 from assumption that HTTP logging is enough.

---
 src/allmydata/storage/http_client.py | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index 44ba64363..fcfc5bff3 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -746,22 +746,18 @@ class StorageClientImmutables(object):
         """
         Return the set of shares for a given storage index.
         """
-        with start_action(
-            action_type="allmydata:storage:http-client:immutable:list-shares",
-            storage_index=storage_index,
-        ) as ctx:
-            url = self._client.relative_url(
-                "/storage/v1/immutable/{}/shares".format(_encode_si(storage_index))
-            )
-            response = yield self._client.request(
-                "GET",
-                url,
-            )
-            if response.code == http.OK:
-                body = yield self._client.decode_cbor(response, _SCHEMAS["list_shares"])
-                returnValue(set(body))
-            else:
-                raise ClientException(response.code)
+        url = self._client.relative_url(
+            "/storage/v1/immutable/{}/shares".format(_encode_si(storage_index))
+        )
+        response = yield self._client.request(
+            "GET",
+            url,
+        )
+        if response.code == http.OK:
+            body = yield self._client.decode_cbor(response, _SCHEMAS["list_shares"])
+            returnValue(set(body))
+        else:
+            raise ClientException(response.code)
 
     def advise_corrupt_share(
         self,

From d36adf33a41be87814da8ccdf9a10c21813d53db Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Tue, 28 Mar 2023 13:06:43 -0400
Subject: [PATCH 06/13] Refactor; failing tests for some reason.

---
 src/allmydata/storage/http_server.py | 42 +++++++++++++++-------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py
index 4f970b5a7..517771c02 100644
--- a/src/allmydata/storage/http_server.py
+++ b/src/allmydata/storage/http_server.py
@@ -106,28 +106,31 @@ def _authorization_decorator(required_secrets):
     def decorator(f):
         @wraps(f)
         def route(self, request, *args, **kwargs):
-            if not timing_safe_compare(
-                request.requestHeaders.getRawHeaders("Authorization", [""])[0].encode(
-                    "utf-8"
-                ),
-                swissnum_auth_header(self._swissnum),
-            ):
-                request.setResponseCode(http.UNAUTHORIZED)
-                return b""
-            authorization = request.requestHeaders.getRawHeaders(
-                "X-Tahoe-Authorization", []
-            )
-            try:
-                secrets = _extract_secrets(authorization, required_secrets)
-            except ClientSecretsException:
-                request.setResponseCode(http.BAD_REQUEST)
-                return b"Missing required secrets"
             with start_action(
-                action_type="allmydata:storage:http-server:request",
+                action_type="allmydata:storage:http-server:handle_request",
                 method=request.method,
                 path=request.path,
             ) as ctx:
                 try:
+                    # Check Authorization header:
+                    if not timing_safe_compare(
+                        request.requestHeaders.getRawHeaders("Authorization", [""])[0].encode(
+                            "utf-8"
+                        ),
+                        swissnum_auth_header(self._swissnum),
+                    ):
+                        raise _HTTPError(http.UNAUTHORIZED)
+
+                    # Check secrets:
+                    authorization = request.requestHeaders.getRawHeaders(
+                        "X-Tahoe-Authorization", []
+                    )
+                    try:
+                        secrets = _extract_secrets(authorization, required_secrets)
+                    except ClientSecretsException:
+                        raise _HTTPError(http.BAD_REQUEST)
+
+                    # Run the business logic:
                     result = f(self, request, secrets, *args, **kwargs)
                 except _HTTPError as e:
                     # This isn't an error necessarily for logging purposes,
@@ -136,8 +139,9 @@ def _authorization_decorator(required_secrets):
                     ctx.add_success_fields(response_code=e.code)
                     ctx.finish()
                     raise
-                ctx.add_success_fields(response_code=request.code)
-                return result
+                else:
+                    ctx.add_success_fields(response_code=request.code)
+                    return result
 
         return route
 

From b81fad2970ff3eeea87fa869318dcee1f6db6ce9 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 3 Apr 2023 10:37:49 -0400
Subject: [PATCH 07/13] Make sure tests have the same error testing
 infrastructure as the real thing.

---
 src/allmydata/storage/http_server.py    | 28 ++++++++++++++-----------
 src/allmydata/test/test_storage_http.py |  2 ++
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py
index 517771c02..5ccb43c60 100644
--- a/src/allmydata/storage/http_server.py
+++ b/src/allmydata/storage/http_server.py
@@ -489,6 +489,21 @@ def read_range(
     return d
 
 
+def _add_error_handling(app: Klein):
+    """Add exception handlers to a Klein app."""
+    @app.handle_errors(_HTTPError)
+    def _http_error(_, request, failure):
+        """Handle ``_HTTPError`` exceptions."""
+        request.setResponseCode(failure.value.code)
+        return b""
+
+    @app.handle_errors(CDDLValidationError)
+    def _cddl_validation_error(_, request, failure):
+        """Handle CDDL validation errors."""
+        request.setResponseCode(http.BAD_REQUEST)
+        return str(failure.value).encode("utf-8")
+
+
 class HTTPServer(object):
     """
     A HTTP interface to the storage server.
@@ -496,18 +511,7 @@ class HTTPServer(object):
 
     _app = Klein()
     _app.url_map.converters["storage_index"] = StorageIndexConverter
-
-    @_app.handle_errors(_HTTPError)
-    def _http_error(self, request, failure):
-        """Handle ``_HTTPError`` exceptions."""
-        request.setResponseCode(failure.value.code)
-        return b""
-
-    @_app.handle_errors(CDDLValidationError)
-    def _cddl_validation_error(self, request, failure):
-        """Handle CDDL validation errors."""
-        request.setResponseCode(http.BAD_REQUEST)
-        return str(failure.value).encode("utf-8")
+    _add_error_handling(_app)
 
     def __init__(
         self,
diff --git a/src/allmydata/test/test_storage_http.py b/src/allmydata/test/test_storage_http.py
index eb5bcd4db..19529cd0e 100644
--- a/src/allmydata/test/test_storage_http.py
+++ b/src/allmydata/test/test_storage_http.py
@@ -54,6 +54,7 @@ from ..storage.http_server import (
     ClientSecretsException,
     _authorized_route,
     StorageIndexConverter,
+    _add_error_handling
 )
 from ..storage.http_client import (
     StorageClient,
@@ -253,6 +254,7 @@ class TestApp(object):
 
     clock: IReactorTime
     _app = Klein()
+    _add_error_handling(_app)
     _swissnum = SWISSNUM_FOR_TEST  # Match what the test client is using
 
     @_authorized_route(_app, {Secrets.UPLOAD}, "/upload_secret", methods=["GET"])

From 41939e2b286fedca55af7ad202739751c40d7f3d Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 3 Apr 2023 11:11:24 -0400
Subject: [PATCH 08/13] Add some type annotations.

---
 src/allmydata/storage/http_client.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index fcfc5bff3..6450050a9 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -341,7 +341,7 @@ class StorageClient(object):
         https_url = DecodedURL().replace(scheme="https", host=nurl.host, port=nurl.port)
         return cls(https_url, swissnum, treq_client, reactor)
 
-    def relative_url(self, path):
+    def relative_url(self, path: str) -> DecodedURL:
         """Get a URL relative to the base URL."""
         return self._base_url.click(path)
 
@@ -357,14 +357,14 @@ class StorageClient(object):
 
     def request(
         self,
-        method,
-        url,
-        lease_renew_secret=None,
-        lease_cancel_secret=None,
-        upload_secret=None,
-        write_enabler_secret=None,
-        headers=None,
-        message_to_serialize=None,
+        method: str,
+        url: DecodedURL,
+        lease_renew_secret: Optional[bytes]=None,
+        lease_cancel_secret: Optional[bytes]=None,
+        upload_secret: Optional[bytes]=None,
+        write_enabler_secret: Optional[bytes]=None,
+        headers: Optional[Headers]=None,
+        message_to_serialize: object=None,
         timeout: float = 60,
         **kwargs,
     ):

From 3b3ea5409c7c31b5158c2ec5093d7021a927d2d3 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 3 Apr 2023 11:26:08 -0400
Subject: [PATCH 09/13] Type says we should only pass in DecodedURL.

---
 src/allmydata/test/test_storage_http.py | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/allmydata/test/test_storage_http.py b/src/allmydata/test/test_storage_http.py
index 19529cd0e..ea93ad360 100644
--- a/src/allmydata/test/test_storage_http.py
+++ b/src/allmydata/test/test_storage_http.py
@@ -54,7 +54,7 @@ from ..storage.http_server import (
     ClientSecretsException,
     _authorized_route,
     StorageIndexConverter,
-    _add_error_handling
+    _add_error_handling,
 )
 from ..storage.http_client import (
     StorageClient,
@@ -348,7 +348,7 @@ class CustomHTTPServerTests(SyncTestCase):
         response = result_of(
             self.client.request(
                 "GET",
-                "http://127.0.0.1/upload_secret",
+                DecodedURL.from_text("http://127.0.0.1/upload_secret"),
             )
         )
         self.assertEqual(response.code, 400)
@@ -356,7 +356,9 @@ class CustomHTTPServerTests(SyncTestCase):
         # With secret, we're good.
         response = result_of(
             self.client.request(
-                "GET", "http://127.0.0.1/upload_secret", upload_secret=b"MAGIC"
+                "GET",
+                DecodedURL.from_text("http://127.0.0.1/upload_secret"),
+                upload_secret=b"MAGIC",
             )
         )
         self.assertEqual(response.code, 200)
@@ -380,7 +382,7 @@ class CustomHTTPServerTests(SyncTestCase):
             response = result_of(
                 self.client.request(
                     "GET",
-                    f"http://127.0.0.1/bytes/{length}",
+                    DecodedURL.from_text(f"http://127.0.0.1/bytes/{length}"),
                 )
             )
 
@@ -401,7 +403,7 @@ class CustomHTTPServerTests(SyncTestCase):
             response = result_of(
                 self.client.request(
                     "GET",
-                    f"http://127.0.0.1/bytes/{length}",
+                    DecodedURL.from_text(f"http://127.0.0.1/bytes/{length}"),
                 )
             )
 
@@ -416,7 +418,7 @@ class CustomHTTPServerTests(SyncTestCase):
         response = result_of(
             self.client.request(
                 "GET",
-                "http://127.0.0.1/slowly_never_finish_result",
+                DecodedURL.from_text("http://127.0.0.1/slowly_never_finish_result"),
             )
         )
 
@@ -444,7 +446,7 @@ class CustomHTTPServerTests(SyncTestCase):
         response = result_of(
             self.client.request(
                 "GET",
-                "http://127.0.0.1/die",
+                DecodedURL.from_text("http://127.0.0.1/die"),
             )
         )
 
@@ -461,6 +463,7 @@ class Reactor(Clock):
 
     Advancing the clock also runs any callbacks scheduled via callFromThread.
     """
+
     def __init__(self):
         Clock.__init__(self)
         self._queue = Queue()
@@ -501,7 +504,9 @@ class HttpTestFixture(Fixture):
         self.storage_server = StorageServer(
             self.tempdir.path, b"\x00" * 20, clock=self.clock
         )
-        self.http_server = HTTPServer(self.clock, self.storage_server, SWISSNUM_FOR_TEST)
+        self.http_server = HTTPServer(
+            self.clock, self.storage_server, SWISSNUM_FOR_TEST
+        )
         self.treq = StubTreq(self.http_server.get_resource())
         self.client = StorageClient(
             DecodedURL.from_text("http://127.0.0.1"),

From 57ec669e1e70dc0fd6be4b6ddfbe3fe0f32221de Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 3 Apr 2023 11:29:57 -0400
Subject: [PATCH 10/13] Add logging for request().

---
 src/allmydata/storage/http_client.py | 58 ++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index 6450050a9..cbd4634b4 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -19,7 +19,7 @@ from collections_extended import RangeMap
 from werkzeug.datastructures import Range, ContentRange
 from twisted.web.http_headers import Headers
 from twisted.web import http
-from twisted.web.iweb import IPolicyForHTTPS
+from twisted.web.iweb import IPolicyForHTTPS, IResponse
 from twisted.internet.defer import inlineCallbacks, returnValue, fail, Deferred, succeed
 from twisted.internet.interfaces import (
     IOpenSSLClientConnectionCreator,
@@ -355,19 +355,20 @@ class StorageClient(object):
         )
         return headers
 
-    def request(
+    @async_to_deferred
+    async def request(
         self,
         method: str,
-        url: DecodedURL,
-        lease_renew_secret: Optional[bytes]=None,
-        lease_cancel_secret: Optional[bytes]=None,
-        upload_secret: Optional[bytes]=None,
-        write_enabler_secret: Optional[bytes]=None,
-        headers: Optional[Headers]=None,
-        message_to_serialize: object=None,
+        url: str,
+        lease_renew_secret: Optional[bytes] = None,
+        lease_cancel_secret: Optional[bytes] = None,
+        upload_secret: Optional[bytes] = None,
+        write_enabler_secret: Optional[bytes] = None,
+        headers: Optional[Headers] = None,
+        message_to_serialize: object = None,
         timeout: float = 60,
         **kwargs,
-    ):
+    ) -> Deferred[IResponse]:
         """
         Like ``treq.request()``, but with optional secrets that get translated
         into corresponding HTTP headers.
@@ -377,6 +378,41 @@ class StorageClient(object):
 
         Default timeout is 60 seconds.
         """
+        with start_action(
+            action_type="allmydata:storage:http-client:request",
+            method=method,
+            url=url.to_text(),
+            timeout=timeout,
+        ) as ctx:
+            response = await self._request(
+                method,
+                url,
+                lease_renew_secret,
+                lease_cancel_secret,
+                upload_secret,
+                write_enabler_secret,
+                headers,
+                message_to_serialize,
+                timeout,
+                **kwargs,
+            )
+            ctx.add_success_fields(response_code=response.code)
+            return response
+
+    async def _request(
+        self,
+        method: str,
+        url: str,
+        lease_renew_secret: Optional[bytes] = None,
+        lease_cancel_secret: Optional[bytes] = None,
+        upload_secret: Optional[bytes] = None,
+        write_enabler_secret: Optional[bytes] = None,
+        headers: Optional[Headers] = None,
+        message_to_serialize: object = None,
+        timeout: float = 60,
+        **kwargs,
+    ) -> IResponse:
+        """The implementation of request()."""
         headers = self._get_headers(headers)
 
         # Add secrets:
@@ -407,7 +443,7 @@ class StorageClient(object):
             kwargs["data"] = dumps(message_to_serialize)
             headers.addRawHeader("Content-Type", CBOR_MIME_TYPE)
 
-        return self._treq.request(
+        return await self._treq.request(
             method, url, headers=headers, timeout=timeout, **kwargs
         )
 

From 5e3fa04a3a6e0ae2210fcdc49347e18c3e384ec2 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 3 Apr 2023 11:30:22 -0400
Subject: [PATCH 11/13] Reformat with black.

---
 src/allmydata/storage/http_client.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index cbd4634b4..cead33732 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -488,12 +488,14 @@ class StorageClientGeneral(object):
         # Add some features we know are true because the HTTP API
         # specification requires them and because other parts of the storage
         # client implementation assumes they will be present.
-        decoded_response[b"http://allmydata.org/tahoe/protocols/storage/v1"].update({
-            b'tolerates-immutable-read-overrun': True,
-            b'delete-mutable-shares-with-zero-length-writev': True,
-            b'fills-holes-with-zero-bytes': True,
-            b'prevents-read-past-end-of-share-data': True,
-        })
+        decoded_response[b"http://allmydata.org/tahoe/protocols/storage/v1"].update(
+            {
+                b"tolerates-immutable-read-overrun": True,
+                b"delete-mutable-shares-with-zero-length-writev": True,
+                b"fills-holes-with-zero-bytes": True,
+                b"prevents-read-past-end-of-share-data": True,
+            }
+        )
         returnValue(decoded_response)
 
     @inlineCallbacks

From e19aeb5aea2bd57bfb2de3bc6f7f87a9097723c6 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Mon, 3 Apr 2023 11:40:48 -0400
Subject: [PATCH 12/13] Correct the annotation.

---
 src/allmydata/storage/http_client.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index cead33732..bd9e3fc39 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -359,7 +359,7 @@ class StorageClient(object):
     async def request(
         self,
         method: str,
-        url: str,
+        url: DecodedURL,
         lease_renew_secret: Optional[bytes] = None,
         lease_cancel_secret: Optional[bytes] = None,
         upload_secret: Optional[bytes] = None,
@@ -402,7 +402,7 @@ class StorageClient(object):
     async def _request(
         self,
         method: str,
-        url: str,
+        url: DecodedURL,
         lease_renew_secret: Optional[bytes] = None,
         lease_cancel_secret: Optional[bytes] = None,
         upload_secret: Optional[bytes] = None,

From 4d4649f5c24ff89f1b538a09740eaffefea80dd2 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 10 Apr 2023 11:28:26 -0400
Subject: [PATCH 13/13] Apply suggestions from code review

Co-authored-by: Jean-Paul Calderone <exarkun@twistedmatrix.com>
---
 src/allmydata/storage/http_client.py | 2 +-
 src/allmydata/storage/http_server.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index bd9e3fc39..ea142ed85 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -368,7 +368,7 @@ class StorageClient(object):
         message_to_serialize: object = None,
         timeout: float = 60,
         **kwargs,
-    ) -> Deferred[IResponse]:
+    ) -> IResponse:
         """
         Like ``treq.request()``, but with optional secrets that get translated
         into corresponding HTTP headers.
diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py
index 5ccb43c60..8647274f8 100644
--- a/src/allmydata/storage/http_server.py
+++ b/src/allmydata/storage/http_server.py
@@ -107,7 +107,7 @@ def _authorization_decorator(required_secrets):
         @wraps(f)
         def route(self, request, *args, **kwargs):
             with start_action(
-                action_type="allmydata:storage:http-server:handle_request",
+                action_type="allmydata:storage:http-server:handle-request",
                 method=request.method,
                 path=request.path,
             ) as ctx: