Repository: filipre/signalbot

Issue:
[BUG] https not supported
Thanks for the cool project and providing the library. 
I use the rest service for signal via https. Unfortunately, I get an error message. It works perfectly with http.

Discussion:
@Era-Dorta: The URIs are hardcoded in https://github.com/filipre/signalbot/blob/master/signalbot/api.py#L222-L244

I think a good solution is to call the health check at `http://{self.signal_service}/v1/health` both for `http` and `https`. Then set the URIs accordingly  and gracefully crash if the health check is not reachable or faulty.@Era-Dorta: I've implemented the changes to support https in #113 , I haven't figured out yet how to launch signal-cli-rest-api in https mode, can you please test it and let me know how it goes?

Solution patch:
diff --git a/.github/workflows/deploy-pr.yaml b/.github/workflows/deploy-pr-command.yaml
similarity index 71%
rename from .github/workflows/deploy-pr.yaml
rename to .github/workflows/deploy-pr-command.yaml
index cf59a49..aa9f8df 100644
--- a/.github/workflows/deploy-pr.yaml
+++ b/.github/workflows/deploy-pr-command.yaml
@@ -1,48 +1,15 @@
 name: Deploy Pull Request
-
 on:
-  issue_comment:
-    types: [created]
+  repository_dispatch:
+    types: [deploy-command]
 
 jobs:
-  deploy-check:
-    runs-on: ubuntu-latest
-    steps:
-      - name: "Check if user has write access"
-        uses: actions-cool/check-user-permission@main
-        id: checkUser
-        with:
-          require: "write"
-      - name: Acknowledge deployment request to commenter
-        if: steps.checkUser.outputs.require-result == 'true'
-        id: checkCommand
-        uses: khan/pull-request-comment-trigger@master
-        with:
-          trigger: "/deploy"
-          reaction: rocket
-        env:
-          GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
-    outputs:
-      triggered: ${{ steps.checkCommand.outputs.triggered }}
-
   publish-package:
     name: Publish Package
     runs-on: ubuntu-latest
-    needs: deploy-check
-    if: needs.deploy-check.outputs.triggered == 'true'
     environment:
       name: test.pypi.org
     steps:
-      - name: Get pull request ref
-        id: get_pull_request_ref
-        uses: octokit/request-action@v2.x
-        with:
-          route: GET /repos/:repository/pulls/:issue_id
-          repository: ${{ github.repository }}
-          issue_id: ${{ github.event.issue.number }}
-        env:
-          GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
-
       - name: Create deployment
         id: create_deployment
         uses: octokit/request-action@v2.x
@@ -50,7 +17,7 @@ jobs:
           route: POST /repos/:repository/deployments
         env:
           INPUT_REPOSITORY: ${{ github.repository }}
-          INPUT_REF: ${{ fromJson(steps.get_pull_request_ref.outputs.data).head.ref }}
+          INPUT_REF: ${{ github.event.client_payload.pull_request.head.ref }}
           INPUT_ENVIRONMENT: test.pypi.org
           INPUT_AUTO_MERGE: false
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
@@ -60,18 +27,18 @@ jobs:
         uses: octokit/request-action@v2.x
         with:
           route: POST /repos/:repository/deployments/:deployment/statuses
-          repository: ${{ github.repository }}
           deployment: ${{ fromJson(steps.create_deployment.outputs.data).id }}
           environment: test.pypi.org
           log_url: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}
           state: in_progress
         env:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
+          INPUT_REPOSITORY: ${{ github.repository }}
 
       - name: Checkout Code
         uses: actions/checkout@v4
         with:
-          ref: ${{ fromJson(steps.get_pull_request_ref.outputs.data).head.ref }}
+          ref: ${{ github.event.client_payload.pull_request.head.ref }}
 
       - name: Set up Python
         uses: actions/setup-python@v5
@@ -88,7 +55,10 @@ jobs:
 
       - name: Create custom version
         id: create-custom-version
-        run: echo "{pr_version}=$(poetry version -s)a$(date +%s)" >> $GITHUB_OUTPUT
+        run: |
+          pr_version=$(poetry version -s)a$(date +%s)
+          echo "The custom version is ${pr_version}"
+          echo "pr_version=${pr_version}" >> "$GITHUB_OUTPUT"
 
       - name: Set new version
         run: poetry version ${{ steps.create-custom-version.outputs.pr_version }}
@@ -107,7 +77,7 @@ jobs:
       - name: Write comment in Pull Request
         uses: peter-evans/create-or-update-comment@v4
         with:
-          issue-number: ${{ github.event.issue.number }}
+          issue-number: ${{ github.event.client_payload.pull_request.number }}
           body: |
             Pull Request has been deployed on test.pypi.org: [${{ steps.create-custom-version.outputs.pr_version }}](https://test.pypi.org/project/signalbot/${{ steps.create-custom-version.outputs.pr_version }}/) :rocket: You can test the package by running
             ```bash
@@ -119,7 +89,6 @@ jobs:
         uses: octokit/request-action@v2.x
         with:
           route: POST /repos/:repository/deployments/:deployment/statuses
-          repository: ${{ github.repository }}
           deployment: ${{ fromJson(steps.create_deployment.outputs.data).id }}
           environment: test.pypi.org
           environment_url: https://test.pypi.org/project/signalbot/${{ steps.create-custom-version.outputs.pr_version }}/
@@ -127,6 +96,7 @@ jobs:
           state: success
         env:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
+          INPUT_REPOSITORY: ${{ github.repository }}
 
       - name: Set deployment status to failure
         id: failed_deployment
@@ -134,7 +104,6 @@ jobs:
         if: failure()
         with:
           route: POST /repos/:repository/deployments/:deployment/statuses
-          repository: ${{ github.repository }}
           deployment: ${{ fromJson(steps.create_deployment.outputs.data).id }}
           environment: test.pypi.org
           environment_url: https://test.pypi.org/project/signalbot/${{ steps.create-custom-version.outputs.pr_version }}/
@@ -142,3 +111,4 @@ jobs:
           state: failure
         env:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
+          INPUT_REPOSITORY: ${{ github.repository }}
diff --git a/.github/workflows/deploy-pr-dispatch.yaml b/.github/workflows/deploy-pr-dispatch.yaml
new file mode 100644
index 0000000..49752d7
--- /dev/null
+++ b/.github/workflows/deploy-pr-dispatch.yaml
@@ -0,0 +1,23 @@
+name: Deploy Pull Request Dispatch
+
+on:
+  issue_comment:
+    types: [created]
+
+jobs:
+  deploy-check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check if user has write access
+        uses: actions-cool/check-user-permission@main
+        id: checkUser
+        with:
+          require: "write"
+      - name: Trigger the deploy-pr-command workflow and add a reaction to the comment
+        if: steps.checkUser.outputs.require-result == 'true'
+        id: checkCommand
+        uses: peter-evans/slash-command-dispatch@v4
+        with:
+          commands: |
+            deploy
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/signalbot/api.py b/signalbot/api.py
index ab20d47..7749a26 100644
--- a/signalbot/api.py
+++ b/signalbot/api.py
@@ -11,14 +11,17 @@ class SignalAPI:
         signal_service: str,
         phone_number: str,
     ):
-        self.signal_service = signal_service
         self.phone_number = phone_number
+        self._signal_api_uris = SignalAPIURIs(
+            signal_service=signal_service,
+            phone_number=phone_number,
+        )
 
         # self.session = aiohttp.ClientSession()
 
     async def receive(self):
         try:
-            uri = self._receive_ws_uri()
+            uri = self._signal_api_uris.receive_ws_uri()
             self.connection = websockets.connect(uri, ping_interval=None)
             async with self.connection as websocket:
                 async for raw_message in websocket:
@@ -39,7 +42,7 @@ class SignalAPI:
         mentions: list[dict[str, Any]] | None = None,
         text_mode: str = None,
     ) -> aiohttp.ClientResponse:
-        uri = self._send_rest_uri()
+        uri = self._signal_api_uris.send_rest_uri()
         if base64_attachments is None:
             base64_attachments = []
 
@@ -78,7 +81,7 @@ class SignalAPI:
     async def react(
         self, recipient: str, reaction: str, target_author: str, timestamp: int
     ) -> aiohttp.ClientResponse:
-        uri = self._react_rest_uri()
+        uri = self._signal_api_uris.react_rest_uri()
         payload = {
             "recipient": recipient,
             "reaction": reaction,
@@ -97,7 +100,7 @@ class SignalAPI:
             raise ReactionError
 
     async def start_typing(self, receiver: str):
-        uri = self._typing_indicator_uri()
+        uri = self._signal_api_uris.typing_indicator_uri()
         payload = {
             "recipient": receiver,
         }
@@ -113,7 +116,7 @@ class SignalAPI:
             raise StartTypingError
 
     async def stop_typing(self, receiver: str):
-        uri = self._typing_indicator_uri()
+        uri = self._signal_api_uris.typing_indicator_uri()
         payload = {
             "recipient": receiver,
         }
@@ -129,7 +132,7 @@ class SignalAPI:
             raise StopTypingError
 
     async def get_groups(self) -> list[dict[str, Any]]:
-        uri = self._groups_uri()
+        uri = self._signal_api_uris.groups_uri()
         try:
             async with aiohttp.ClientSession() as session:
                 resp = await session.get(uri)
@@ -142,7 +145,7 @@ class SignalAPI:
             raise GroupsError
 
     async def get_attachment(self, attachment_id: str) -> str:
-        uri = f"{self._attachment_rest_uri()}/{attachment_id}"
+        uri = f"{self._signal_api_uris.attachment_rest_uri()}/{attachment_id}"
         try:
             async with aiohttp.ClientSession() as session:
                 resp = await session.get(uri)
@@ -160,7 +163,7 @@ class SignalAPI:
         return base64_string
 
     async def delete_attachment(self, attachment_id: str) -> str:
-        uri = f"{self._attachment_rest_uri()}/{attachment_id}"
+        uri = f"{self._signal_api_uris.attachment_rest_uri()}/{attachment_id}"
         try:
             async with aiohttp.ClientSession() as session:
                 resp = await session.delete(uri)
@@ -178,7 +181,7 @@ class SignalAPI:
         expiration_in_seconds: Optional[int] = None,
         name: Optional[str] = None,
     ) -> None:
-        uri = self._contacts_uri()
+        uri = self._signal_api_uris.contacts_uri()
         payload = {"recipient": receiver}
 
         if expiration_in_seconds is not None:
@@ -206,7 +209,7 @@ class SignalAPI:
         expiration_in_seconds: Optional[int] = None,
         name: Optional[str] = None,
     ) -> None:
-        uri = self._group_id_uri(group_id)
+        uri = self._signal_api_uris.group_id_uri(group_id)
         payload = {}
 
         if base64_avatar is not None:
@@ -232,29 +235,74 @@ class SignalAPI:
         ):
             raise ContactUpdateError
 
-    def _attachment_rest_uri(self):
-        return f"http://{self.signal_service}/v1/attachments"
+    async def health_check(self) -> aiohttp.ClientResponse:
+        uri = self._signal_api_uris.health_check_uri()
+        try:
+            async with aiohttp.ClientSession() as session:
+                resp = await session.get(uri)
+                resp.raise_for_status()
+                return resp
+        except (
+            aiohttp.ClientError,
+            aiohttp.http_exceptions.HttpProcessingError,
+        ):
+            raise HealthCheckError
+
+    async def check_signal_service(self) -> bool:
+        self._signal_api_uris.use_https = True
+        try:
+            return (await self.health_check()).status == 204
+        except HealthCheckError:
+            self._signal_api_uris.use_https = False
+            try:
+                return (await self.health_check()).status == 204
+            except HealthCheckError as e:
+                return False
 
-    def _receive_ws_uri(self):
-        return f"ws://{self.signal_service}/v1/receive/{self.phone_number}"
 
-    def _send_rest_uri(self):
-        return f"http://{self.signal_service}/v2/send"
+class SignalAPIURIs:
 
-    def _react_rest_uri(self):
-        return f"http://{self.signal_service}/v1/reactions/{self.phone_number}"
+    def __init__(self, signal_service: str, phone_number: str, use_https: bool = True):
+        self.signal_service = signal_service
+        self.phone_number = phone_number
+        self.use_https = use_https
 
-    def _typing_indicator_uri(self):
-        return f"http://{self.signal_service}/v1/typing-indicator/{self.phone_number}"
+    @property
+    def https_or_http(self) -> str:
+        return "https" if self.use_https else "http"
 
-    def _groups_uri(self):
-        return f"http://{self.signal_service}/v1/groups/{self.phone_number}"
+    @property
+    def wss_or_ws(self) -> str:
+        return "wss" if self.use_https else "ws"
 
-    def _group_id_uri(self, group_id: str):
-        return self._groups_uri() + "/" + group_id
+    def attachment_rest_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v1/attachments"
 
-    def _contacts_uri(self):
-        return f"http://{self.signal_service}/v1/contacts/{self.phone_number}"
+    def receive_ws_uri(self):
+        return (
+            f"{self.wss_or_ws}://{self.signal_service}/v1/receive/{self.phone_number}"
+        )
+
+    def send_rest_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v2/send"
+
+    def react_rest_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v1/reactions/{self.phone_number}"
+
+    def typing_indicator_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v1/typing-indicator/{self.phone_number}"
+
+    def groups_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v1/groups/{self.phone_number}"
+
+    def group_id_uri(self, group_id: str):
+        return self.groups_uri() + "/" + group_id
+
+    def contacts_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v1/contacts/{self.phone_number}"
+
+    def health_check_uri(self):
+        return f"{self.https_or_http}://{self.signal_service}/v1/health"
 
 
 class ReceiveMessagesError(Exception):
@@ -291,3 +339,11 @@ class GetAttachmentError(Exception):
 
 class ContactUpdateError(Exception):
     pass
+
+
+class HealthCheckError(Exception):
+    pass
+
+
+class SignalServiceConnectionError(Exception):
+    pass
diff --git a/signalbot/bot.py b/signalbot/bot.py
index c81f17b..b90ab76 100644
--- a/signalbot/bot.py
+++ b/signalbot/bot.py
@@ -30,12 +30,15 @@ class SignalBot:
         """SignalBot
 
         Example Config:
-        ===============
+        ======= Mandatory fields ========
         signal_service: "127.0.0.1:8080"
         phone_number: "+49123456789"
+
+        ======= Optional fields ========
         storage:
             redis_host: "redis"
             redis_port: 6379
+        retry_interval: 1
         """
         self.config = config
 
@@ -186,10 +189,16 @@ class SignalBot:
             self.commands.append((command, contacts, group_ids, f))
 
     async def _async_post_init(self):
+        await self._check_signal_service()
         await self._detect_groups()
         await self._resolve_commands()
         await self._produce_consume_messages()
 
+    async def _check_signal_service(self):
+        while (await self._signal.check_signal_service()) is False:
+            logging.error("Cannot connect to the signal-cli-rest-api service, retrying")
+            await asyncio.sleep(self.config.get("retry_interval", 1))
+
     def _store_reference_to_task(self, task: asyncio.Task):
         # Keep a hard reference to the tasks, fixes Ruff's RUF006 rule
         self._running_tasks.add(task)


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 9ad7845..9c8492e 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -50,18 +50,18 @@ class TestAPI(unittest.IsolatedAsyncioTestCase):
             self.assertEqual(messages[i], results[i])
 
     def test_receive_uri(self):
-        expected_uri = f"ws://{self.signal_service}/v1/receive/{self.phone_number}"
-        actual_uri = self.signal_api._receive_ws_uri()
+        expected_uri = f"wss://{self.signal_service}/v1/receive/{self.phone_number}"
+        actual_uri = self.signal_api._signal_api_uris.receive_ws_uri()
         self.assertEqual(actual_uri, expected_uri)
 
     def test_send_uri(self):
-        expected_uri = f"http://{self.signal_service}/v2/send"
-        actual_uri = self.signal_api._send_rest_uri()
+        expected_uri = f"https://{self.signal_service}/v2/send"
+        actual_uri = self.signal_api._signal_api_uris.send_rest_uri()
         self.assertEqual(actual_uri, expected_uri)
 
     def test_attachment_rest_uri(self):
-        expected_uri = f"http://{self.signal_service}/v1/attachments"
-        actual_uri = self.signal_api._attachment_rest_uri()
+        expected_uri = f"https://{self.signal_service}/v1/attachments"
+        actual_uri = self.signal_api._signal_api_uris.attachment_rest_uri()
         self.assertEqual(actual_uri, expected_uri)