From 6a48b66a96c001d5b652f25ad78866cb04f56421 Mon Sep 17 00:00:00 2001
From: Chenna Keshava <ckeshavabs@gmail.com>
Date: Wed, 14 Feb 2024 16:12:21 -0800
Subject: [PATCH 1/5] setup uniform interfaces for all Client._request_impl
 overridden functions TODO: set up integration tests to ensure that high
 latency commands (like ledger) do not timeout within the default 10 seconds

---
 xrpl/asyncio/clients/async_websocket_client.py |  8 +++++---
 xrpl/asyncio/clients/client.py                 | 16 +++++++++++++++-
 xrpl/asyncio/clients/json_rpc_base.py          | 14 ++++++++------
 xrpl/asyncio/clients/websocket_base.py         | 16 ++++++++++++----
 xrpl/clients/websocket_client.py               |  6 ++++--
 5 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/xrpl/asyncio/clients/async_websocket_client.py b/xrpl/asyncio/clients/async_websocket_client.py
index 8aa024bb5..3ef883b41 100644
--- a/xrpl/asyncio/clients/async_websocket_client.py
+++ b/xrpl/asyncio/clients/async_websocket_client.py
@@ -3,7 +3,7 @@
 
 from collections.abc import AsyncIterator
 from types import TracebackType
-from typing import Any, Dict, Type
+from typing import Any, Dict, Optional, Type
 
 from xrpl.asyncio.clients.async_client import AsyncClient
 from xrpl.asyncio.clients.exceptions import XRPLWebsocketException
@@ -262,7 +262,9 @@ async def send(self: AsyncWebsocketClient, request: Request) -> None:
             raise XRPLWebsocketException("Websocket is not open")
         await self._do_send(request)
 
-    async def _request_impl(self: WebsocketBase, request: Request) -> Response:
+    async def _request_impl(
+        self: WebsocketBase, request: Request, timeout: Optional[float] = None
+    ) -> Response:
         """
         ``_request_impl`` implementation for async websocket.
 
@@ -280,4 +282,4 @@ async def _request_impl(self: WebsocketBase, request: Request) -> Response:
         """
         if not self.is_open():
             raise XRPLWebsocketException("Websocket is not open")
-        return await self._do_request_impl(request)
+        return await self._do_request_impl(request, timeout)
diff --git a/xrpl/asyncio/clients/client.py b/xrpl/asyncio/clients/client.py
index 15acadd2c..6b1315bcd 100644
--- a/xrpl/asyncio/clients/client.py
+++ b/xrpl/asyncio/clients/client.py
@@ -4,9 +4,15 @@
 from abc import ABC, abstractmethod
 from typing import Optional
 
+from typing_extensions import Final
+
 from xrpl.models.requests.request import Request
 from xrpl.models.response import Response
 
+# The default request timeout duration. If you need more time, please pass the required
+# value into the Client._request_impl function
+_TIMEOUT: Final[float] = 10.0
+
 
 class Client(ABC):
     """
@@ -27,7 +33,9 @@ def __init__(self: Client, url: str) -> None:
         self.build_version: Optional[str] = None
 
     @abstractmethod
-    async def _request_impl(self: Client, request: Request) -> Response:
+    async def _request_impl(
+        self: Client, request: Request, timeout: Optional[float] = _TIMEOUT
+    ) -> Response:
         """
         This is the actual driver for a given Client's request. It must be
         async because all of the helper functions in this library are
@@ -35,6 +43,12 @@ async def _request_impl(self: Client, request: Request) -> Response:
 
         Arguments:
             request: An object representing information about a rippled request.
+            timeout: The maximum tolerable delay on waiting for a response.
+                Note: Optional is used in the type in order to honor the existing
+                behavior in certain overridden functions. WebsocketBase.do_request_impl
+                waits indefinitely for the completion of a request, whereas
+                JsonRpcBase._request_impl waits for 10 seconds before timing out a
+                request.
 
         Returns:
             The response from the server, as a Response object.
diff --git a/xrpl/asyncio/clients/json_rpc_base.py b/xrpl/asyncio/clients/json_rpc_base.py
index 887b6e19e..23d7897ea 100644
--- a/xrpl/asyncio/clients/json_rpc_base.py
+++ b/xrpl/asyncio/clients/json_rpc_base.py
@@ -2,18 +2,16 @@
 from __future__ import annotations
 
 from json import JSONDecodeError
+from typing import Optional
 
 from httpx import AsyncClient
-from typing_extensions import Final
 
-from xrpl.asyncio.clients.client import Client
+from xrpl.asyncio.clients.client import _TIMEOUT, Client
 from xrpl.asyncio.clients.exceptions import XRPLRequestFailureException
 from xrpl.asyncio.clients.utils import json_to_response, request_to_json_rpc
 from xrpl.models.requests.request import Request
 from xrpl.models.response import Response
 
-_TIMEOUT: Final[float] = 10.0
-
 
 class JsonRpcBase(Client):
     """
@@ -22,12 +20,16 @@ class JsonRpcBase(Client):
     :meta private:
     """
 
-    async def _request_impl(self: JsonRpcBase, request: Request) -> Response:
+    async def _request_impl(
+        self: JsonRpcBase, request: Request, timeout: Optional[float] = _TIMEOUT
+    ) -> Response:
         """
         Base ``_request_impl`` implementation for JSON RPC.
 
         Arguments:
             request: An object representing information about a rippled request.
+            timeout: The duration within which we expect to hear a response from the
+            rippled validator.
 
         Returns:
             The response from the server, as a Response object.
@@ -37,7 +39,7 @@ async def _request_impl(self: JsonRpcBase, request: Request) -> Response:
 
         :meta private:
         """
-        async with AsyncClient(timeout=_TIMEOUT) as http_client:
+        async with AsyncClient(timeout=timeout) as http_client:
             response = await http_client.post(
                 self.url,
                 json=request_to_json_rpc(request),
diff --git a/xrpl/asyncio/clients/websocket_base.py b/xrpl/asyncio/clients/websocket_base.py
index 3353db657..34c815b57 100644
--- a/xrpl/asyncio/clients/websocket_base.py
+++ b/xrpl/asyncio/clients/websocket_base.py
@@ -199,7 +199,9 @@ async def _do_pop_message(self: WebsocketBase) -> Dict[str, Any]:
         cast(_MESSAGES_TYPE, self._messages).task_done()
         return msg
 
-    async def _do_request_impl(self: WebsocketBase, request: Request) -> Response:
+    async def _do_request_impl(
+        self: WebsocketBase, request: Request, timeout: Optional[float]
+    ) -> Response:
         """
         Base ``_request_impl`` implementation for websockets.
 
@@ -221,8 +223,14 @@ async def _do_request_impl(self: WebsocketBase, request: Request) -> Response:
 
         # fire-and-forget the send, and await the Future
         asyncio.create_task(self._do_send_no_future(request_with_id))
-        raw_response = await self._open_requests[request_str]
 
-        # remove the resolved Future, hopefully getting it garbage colleted
-        del self._open_requests[request_str]
+        try:
+            raw_response = await asyncio.wait_for(
+                self._open_requests[request_str], timeout
+            )
+        finally:
+            # remove the resolved Future, hopefully getting it garbage colleted
+            # Ensure the request is removed whether it times out or not
+            del self._open_requests[request_str]
+
         return websocket_to_response(raw_response)
diff --git a/xrpl/clients/websocket_client.py b/xrpl/clients/websocket_client.py
index 2eb446a75..1322aa1b4 100644
--- a/xrpl/clients/websocket_client.py
+++ b/xrpl/clients/websocket_client.py
@@ -200,7 +200,9 @@ def send(self: WebsocketClient, request: Request) -> None:
             self._do_send(request), cast(asyncio.AbstractEventLoop, self._loop)
         ).result()
 
-    async def _request_impl(self: WebsocketClient, request: Request) -> Response:
+    async def _request_impl(
+        self: WebsocketClient, request: Request, timeout: Optional[float] = None
+    ) -> Response:
         """
         ``_request_impl`` implementation for sync websockets that ensures the
         ``WebsocketBase._do_request_impl`` implementation is run on the other thread.
@@ -232,6 +234,6 @@ async def _request_impl(self: WebsocketClient, request: Request) -> Response:
         # completely block the main thread until completed,
         # just as if it were not async.
         return asyncio.run_coroutine_threadsafe(
-            self._do_request_impl(request),
+            self._do_request_impl(request, timeout),
             cast(asyncio.AbstractEventLoop, self._loop),
         ).result()

From 4e4277d017a9dbd34905b43818b155b51f3a3daa Mon Sep 17 00:00:00 2001
From: Chenna Keshava <ckeshavabs@gmail.com>
Date: Thu, 15 Feb 2024 10:45:36 -0800
Subject: [PATCH 2/5] update CHANGELOG

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index df1871754..38101ec02 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added support for `XChainModifyBridge` flag maps (fixing an issue with `NFTokenCreateOffer` flag names)
 - Fixed `XChainModifyBridge` validation to allow just clearing of `MinAccountCreateAmount`
 - Currency codes with special characters not being allowed by IssuedCurrency objects.
+- Specify custom timeout parameter for requests.
 
 ## [2.5.0] - 2023-11-30
 

From 101eaca9fb1001a8c41f74c10caf30112d64614f Mon Sep 17 00:00:00 2001
From: Chenna Keshava <ckeshavabs@gmail.com>
Date: Thu, 15 Feb 2024 15:30:41 -0800
Subject: [PATCH 3/5] ensure that timeout is a keyword-only arg. this avoids
 mistaken usage of this parameter

---
 xrpl/asyncio/clients/async_websocket_client.py | 2 +-
 xrpl/asyncio/clients/client.py                 | 2 +-
 xrpl/asyncio/clients/json_rpc_base.py          | 2 +-
 xrpl/clients/websocket_client.py               | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xrpl/asyncio/clients/async_websocket_client.py b/xrpl/asyncio/clients/async_websocket_client.py
index 3ef883b41..553aa821c 100644
--- a/xrpl/asyncio/clients/async_websocket_client.py
+++ b/xrpl/asyncio/clients/async_websocket_client.py
@@ -263,7 +263,7 @@ async def send(self: AsyncWebsocketClient, request: Request) -> None:
         await self._do_send(request)
 
     async def _request_impl(
-        self: WebsocketBase, request: Request, timeout: Optional[float] = None
+        self: WebsocketBase, request: Request, *, timeout: Optional[float] = None
     ) -> Response:
         """
         ``_request_impl`` implementation for async websocket.
diff --git a/xrpl/asyncio/clients/client.py b/xrpl/asyncio/clients/client.py
index 6b1315bcd..1e6f910da 100644
--- a/xrpl/asyncio/clients/client.py
+++ b/xrpl/asyncio/clients/client.py
@@ -34,7 +34,7 @@ def __init__(self: Client, url: str) -> None:
 
     @abstractmethod
     async def _request_impl(
-        self: Client, request: Request, timeout: Optional[float] = _TIMEOUT
+        self: Client, request: Request, *, timeout: Optional[float] = _TIMEOUT
     ) -> Response:
         """
         This is the actual driver for a given Client's request. It must be
diff --git a/xrpl/asyncio/clients/json_rpc_base.py b/xrpl/asyncio/clients/json_rpc_base.py
index 23d7897ea..939ecf2c0 100644
--- a/xrpl/asyncio/clients/json_rpc_base.py
+++ b/xrpl/asyncio/clients/json_rpc_base.py
@@ -21,7 +21,7 @@ class JsonRpcBase(Client):
     """
 
     async def _request_impl(
-        self: JsonRpcBase, request: Request, timeout: Optional[float] = _TIMEOUT
+        self: JsonRpcBase, request: Request, *, timeout: Optional[float] = _TIMEOUT
     ) -> Response:
         """
         Base ``_request_impl`` implementation for JSON RPC.
diff --git a/xrpl/clients/websocket_client.py b/xrpl/clients/websocket_client.py
index 1322aa1b4..0ba89a1c8 100644
--- a/xrpl/clients/websocket_client.py
+++ b/xrpl/clients/websocket_client.py
@@ -201,7 +201,7 @@ def send(self: WebsocketClient, request: Request) -> None:
         ).result()
 
     async def _request_impl(
-        self: WebsocketClient, request: Request, timeout: Optional[float] = None
+        self: WebsocketClient, request: Request, *, timeout: Optional[float] = None
     ) -> Response:
         """
         ``_request_impl`` implementation for sync websockets that ensures the

From e0b6bc45696ae4a2dbd6dc780215c22e65245376 Mon Sep 17 00:00:00 2001
From: Chenna Keshava <ckeshavabs@gmail.com>
Date: Thu, 15 Feb 2024 15:31:53 -0800
Subject: [PATCH 4/5] revert the changes to CHANGELOG. _request_impl is an
 internal implementation detail of the request method

---
 CHANGELOG.md | 1 -
 1 file changed, 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 38101ec02..df1871754 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,7 +12,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added support for `XChainModifyBridge` flag maps (fixing an issue with `NFTokenCreateOffer` flag names)
 - Fixed `XChainModifyBridge` validation to allow just clearing of `MinAccountCreateAmount`
 - Currency codes with special characters not being allowed by IssuedCurrency objects.
-- Specify custom timeout parameter for requests.
 
 ## [2.5.0] - 2023-11-30
 

From 70dfb80af3b82a90664da5e1b8e6ef60fe409912 Mon Sep 17 00:00:00 2001
From: Chenna Keshava <ckeshavabs@gmail.com>
Date: Tue, 19 Mar 2024 16:48:45 -0700
Subject: [PATCH 5/5] address PR comments: make timeout a mandatory argument

---
 xrpl/asyncio/clients/async_websocket_client.py |  5 +++--
 xrpl/asyncio/clients/client.py                 | 13 ++++---------
 xrpl/asyncio/clients/json_rpc_base.py          |  5 ++---
 xrpl/asyncio/clients/websocket_base.py         |  2 +-
 xrpl/clients/websocket_client.py               |  3 ++-
 5 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/xrpl/asyncio/clients/async_websocket_client.py b/xrpl/asyncio/clients/async_websocket_client.py
index 553aa821c..345dc2103 100644
--- a/xrpl/asyncio/clients/async_websocket_client.py
+++ b/xrpl/asyncio/clients/async_websocket_client.py
@@ -3,9 +3,10 @@
 
 from collections.abc import AsyncIterator
 from types import TracebackType
-from typing import Any, Dict, Optional, Type
+from typing import Any, Dict, Type
 
 from xrpl.asyncio.clients.async_client import AsyncClient
+from xrpl.asyncio.clients.client import REQUEST_TIMEOUT
 from xrpl.asyncio.clients.exceptions import XRPLWebsocketException
 from xrpl.asyncio.clients.websocket_base import WebsocketBase
 from xrpl.models.requests.request import Request
@@ -263,7 +264,7 @@ async def send(self: AsyncWebsocketClient, request: Request) -> None:
         await self._do_send(request)
 
     async def _request_impl(
-        self: WebsocketBase, request: Request, *, timeout: Optional[float] = None
+        self: WebsocketBase, request: Request, *, timeout: float = REQUEST_TIMEOUT
     ) -> Response:
         """
         ``_request_impl`` implementation for async websocket.
diff --git a/xrpl/asyncio/clients/client.py b/xrpl/asyncio/clients/client.py
index 1e6f910da..15a7cf2b1 100644
--- a/xrpl/asyncio/clients/client.py
+++ b/xrpl/asyncio/clients/client.py
@@ -9,9 +9,9 @@
 from xrpl.models.requests.request import Request
 from xrpl.models.response import Response
 
-# The default request timeout duration. If you need more time, please pass the required
-# value into the Client._request_impl function
-_TIMEOUT: Final[float] = 10.0
+# The default request timeout duration. Set in Client._request_impl to allow more time
+# for longer running commands.
+REQUEST_TIMEOUT: Final[float] = 10.0
 
 
 class Client(ABC):
@@ -34,7 +34,7 @@ def __init__(self: Client, url: str) -> None:
 
     @abstractmethod
     async def _request_impl(
-        self: Client, request: Request, *, timeout: Optional[float] = _TIMEOUT
+        self: Client, request: Request, *, timeout: float = REQUEST_TIMEOUT
     ) -> Response:
         """
         This is the actual driver for a given Client's request. It must be
@@ -44,11 +44,6 @@ async def _request_impl(
         Arguments:
             request: An object representing information about a rippled request.
             timeout: The maximum tolerable delay on waiting for a response.
-                Note: Optional is used in the type in order to honor the existing
-                behavior in certain overridden functions. WebsocketBase.do_request_impl
-                waits indefinitely for the completion of a request, whereas
-                JsonRpcBase._request_impl waits for 10 seconds before timing out a
-                request.
 
         Returns:
             The response from the server, as a Response object.
diff --git a/xrpl/asyncio/clients/json_rpc_base.py b/xrpl/asyncio/clients/json_rpc_base.py
index 939ecf2c0..5b0923bc6 100644
--- a/xrpl/asyncio/clients/json_rpc_base.py
+++ b/xrpl/asyncio/clients/json_rpc_base.py
@@ -2,11 +2,10 @@
 from __future__ import annotations
 
 from json import JSONDecodeError
-from typing import Optional
 
 from httpx import AsyncClient
 
-from xrpl.asyncio.clients.client import _TIMEOUT, Client
+from xrpl.asyncio.clients.client import REQUEST_TIMEOUT, Client
 from xrpl.asyncio.clients.exceptions import XRPLRequestFailureException
 from xrpl.asyncio.clients.utils import json_to_response, request_to_json_rpc
 from xrpl.models.requests.request import Request
@@ -21,7 +20,7 @@ class JsonRpcBase(Client):
     """
 
     async def _request_impl(
-        self: JsonRpcBase, request: Request, *, timeout: Optional[float] = _TIMEOUT
+        self: JsonRpcBase, request: Request, *, timeout: float = REQUEST_TIMEOUT
     ) -> Response:
         """
         Base ``_request_impl`` implementation for JSON RPC.
diff --git a/xrpl/asyncio/clients/websocket_base.py b/xrpl/asyncio/clients/websocket_base.py
index 34c815b57..6c592347a 100644
--- a/xrpl/asyncio/clients/websocket_base.py
+++ b/xrpl/asyncio/clients/websocket_base.py
@@ -200,7 +200,7 @@ async def _do_pop_message(self: WebsocketBase) -> Dict[str, Any]:
         return msg
 
     async def _do_request_impl(
-        self: WebsocketBase, request: Request, timeout: Optional[float]
+        self: WebsocketBase, request: Request, timeout: float
     ) -> Response:
         """
         Base ``_request_impl`` implementation for websockets.
diff --git a/xrpl/clients/websocket_client.py b/xrpl/clients/websocket_client.py
index 0ba89a1c8..8a5febe93 100644
--- a/xrpl/clients/websocket_client.py
+++ b/xrpl/clients/websocket_client.py
@@ -7,6 +7,7 @@
 from types import TracebackType
 from typing import Any, Dict, Iterator, Optional, Type, Union, cast
 
+from xrpl.asyncio.clients.client import REQUEST_TIMEOUT
 from xrpl.asyncio.clients.exceptions import XRPLWebsocketException
 from xrpl.asyncio.clients.websocket_base import WebsocketBase
 from xrpl.clients.sync_client import SyncClient
@@ -201,7 +202,7 @@ def send(self: WebsocketClient, request: Request) -> None:
         ).result()
 
     async def _request_impl(
-        self: WebsocketClient, request: Request, *, timeout: Optional[float] = None
+        self: WebsocketClient, request: Request, *, timeout: float = REQUEST_TIMEOUT
     ) -> Response:
         """
         ``_request_impl`` implementation for sync websockets that ensures the