From: Kyle Fuller <redacted>
Date: Fri, 23 Sep 2022 22:12:47 +0000 (+0100)
Subject: feat: retry on socket error
X-Git-Url: http://git.99rst.org/?a=commitdiff_plain;h=15aa7de3c70f55a36e736ea424f949d67d8d2d0a;p=znc-palaver.git

feat: retry on socket error
---

diff --git a/palaver.cpp b/palaver.cpp
index 8362eda..a548df8 100644
--- a/palaver.cpp
+++ b/palaver.cpp
@@ -188,6 +188,10 @@ public:
 
 	}
 
+	virtual void RetryRequest() {
+
+	}
+
 	void ReadLine(const CString& sData) {
 		CString sLine = sData;
 		sLine.TrimRight("\r\n");
@@ -231,18 +235,37 @@ public:
 
 	void Disconnected() {
 		Close(CSocket::CLT_AFTERWRITE);
+
+		if (m_eState == StatusLine) {
+			// If we've already processed the status line, we've already
+			// fired the handle status (and thus an error is not appropriate).
+			RetryRequest();
+		}
 	}
 
 	void Timeout() {
 		DEBUG("Palaver: HTTP Request timed out '" << m_sHostname << "'");
+
+		if (m_eState == StatusLine) {
+			// If we've already processed the status line, we've already
+			// fired the handle status (and thus an error is not appropriate).
+			RetryRequest();
+		}
 	}
 
 	void ConnectionRefused() {
 		DEBUG("Palaver: Connection refused to '" << m_sHostname << "'");
+		RetryRequest();
 	}
 
 	virtual void SockError(int iErrno, const CString &sDescription) {
 		DEBUG("Palaver: HTTP Request failed '" << m_sHostname << "' - " << sDescription);
+
+		if (m_eState == StatusLine) {
+			// If we've already processed the status line, we;ve already
+			// fired the handle status (and thus an error is not appropriate).
+			RetryRequest();
+		}
 	}
 
 	virtual bool SNIConfigureClient(CS_STRING &sHostname) {
@@ -261,6 +284,7 @@ public:
 	}
 
 	virtual void HandleStatusCode(unsigned int status);
+	virtual void RetryRequest();
 
 private:
 	CString m_sIdentifier;
@@ -1387,6 +1411,15 @@ class PLVRetryTimer : public CTimer {
 	}
 };
 
+void PLVHTTPNotificationSocket::RetryRequest() {
+	RetryStrategy retryStrategy = RetryStrategy();
+	if (retryStrategy.GetMaximumRetryAttempts() > (m_attempts + 1)) {
+		DEBUG("palaver: Retrying failed request");
+		m_pModule->AddTimer(
+			new PLVRetryTimer(m_pModule, retryStrategy.GetDelay(m_attempts + 1), "Request Retry", "Retry a failed pysh notification", m_sIdentifier, m_request, m_attempts + 1)
+		);
+	}
+}
 
 void PLVHTTPNotificationSocket::HandleStatusCode(unsigned int status) {
 	if (status == 401 || status == 404) {
@@ -1398,12 +1431,8 @@ void PLVHTTPNotificationSocket::HandleStatusCode(unsigned int status) {
 	}
 
 	RetryStrategy retryStrategy = RetryStrategy();
-	if (retryStrategy.ShouldRetryRequest(status) && retryStrategy.GetMaximumRetryAttempts() > (m_attempts + 1)) {
-		DEBUG("palaver: Retrying failed request");
-		m_pModule->AddTimer(
-			new PLVRetryTimer(m_pModule, retryStrategy.GetDelay(m_attempts + 1), "Request Retry", "Retry a failed pysh notification", m_sIdentifier, m_request, m_attempts + 1)
-		);
-		return;
+	if (retryStrategy.ShouldRetryRequest(status)) {
+		RetryRequest();
 	}
 }
 
diff --git a/test/test_palaver.py b/test/test_palaver.py
index 827f0f3..7266bc7 100644
--- a/test/test_palaver.py
+++ b/test/test_palaver.py
@@ -381,3 +381,60 @@ async def test_receiving_notification_with_retry_on_server_error(znc):
     await server.wait_closed()
 
     assert connected.requests == 2
+
+
+async def test_receiving_notification_with_retry_on_disconnect(znc):
+    reader, writer = znc
+
+    async def connected(reader, writer):
+        if not hasattr(connected, 'requests'):
+            connected.requests = 1
+            writer.close()
+            return
+
+        headers, body = await read_push_request(reader)
+        assert headers['Authorization'] == 'Bearer abcdefg'
+        assert json.loads(body.decode('utf-8')) == {
+            'badge': 1,
+            'message': 'Test notification',
+            'sender': 'palaver',
+            'network': 'b758eaab1a4611a310642a6e8419fbff'
+        }
+
+        connected.requests += 1
+        writer.write(b'HTTP/1.1 204 No Content\r\n')
+
+        writer.write(b'Connection: close\r\n')
+        writer.write(b'\r\n')
+
+        await writer.drain()
+        writer.close()
+
+    server = await asyncio.start_server(connected, host='127.0.0.1', port=8121)
+    await asyncio.sleep(0.2)
+    addr = server.sockets[0].getsockname()
+    url = f'http://{addr[0]}:{addr[1]}/push'
+
+    writer.write(b'PALAVER IDENTIFY 9167e47b01598af7423e2ecd3d0a3ec4 611d3a30a3d666fc491cdea0d2e1dd6e b758eaab1a4611a310642a6e8419fbff\r\n')
+    await writer.drain()
+
+    line = await reader.readline()
+    assert line == b'PALAVER REQ *\r\n'
+
+    writer.write(b'PALAVER BEGIN 9167e47b01598af7423e2ecd3d0a3ec4 611d3a30a3d666fc491cdea0d2e1dd6e\r\n')
+    writer.write(f'PALAVER SET PUSH-ENDPOINT {url}\r\n'.encode('utf-8'))
+    writer.write(f'PALAVER SET PUSH-TOKEN abcdefg\r\n'.encode('utf-8'))
+    writer.write(b'PALAVER END\r\n')
+    await writer.drain()
+
+    writer.write(b'PRIVMSG *palaver :test\r\n')
+    await writer.drain()
+
+    line = await reader.readline()
+    assert line == b':*palaver!znc@znc.in PRIVMSG admin :Notification sent to 1 clients.\r\n'
+
+    await asyncio.sleep(1.2)
+    server.close()
+    await server.wait_closed()
+
+    assert connected.requests == 2