From e53e01486fbefec7a1ae8ab72d433e63d66aa0bd Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 6 Jul 2025 10:31:05 +0100 Subject: [PATCH 1/2] Add docstring to InvocationCancelledError and expose to new api via exceptions. Also change from SystemExit error as this is not the corrcect error to inherit from. --- src/labthings_fastapi/dependencies/invocation.py | 8 ++++++-- src/labthings_fastapi/exceptions.py | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/labthings_fastapi/dependencies/invocation.py b/src/labthings_fastapi/dependencies/invocation.py index c11917f6..247b2926 100644 --- a/src/labthings_fastapi/dependencies/invocation.py +++ b/src/labthings_fastapi/dependencies/invocation.py @@ -33,8 +33,12 @@ def invocation_logger(id: InvocationID) -> logging.Logger: InvocationLogger = Annotated[logging.Logger, Depends(invocation_logger)] -class InvocationCancelledError(SystemExit): - pass +class InvocationCancelledError(BaseException): + """An invocation was cancelled by the user. + + Note that this inherits from BaseException so won't be caught by + `except Exception`, it must be handled specifically. + """ class CancelEvent(threading.Event): diff --git a/src/labthings_fastapi/exceptions.py b/src/labthings_fastapi/exceptions.py index 26072db1..dfe93ce5 100644 --- a/src/labthings_fastapi/exceptions.py +++ b/src/labthings_fastapi/exceptions.py @@ -1,5 +1,7 @@ """A submodule for custom LabThings-FastAPI Exceptions""" +from .dependencies.invocation import InvocationCancelledError + class NotConnectedToServerError(RuntimeError): """The Thing is not connected to a server @@ -9,3 +11,6 @@ class NotConnectedToServerError(RuntimeError): connected to a ThingServer. A server connection is needed to manage asynchronous behaviour. """ + + +__all__ = ["NotConnectedToServerError", "InvocationCancelledError"] From 0c8cc73c116920ff00fecfb6327a0c4b7e2a709c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 6 Jul 2025 11:39:41 +0100 Subject: [PATCH 2/2] Add more tests for cancelling invocations --- tests/test_action_cancel.py | 205 ++++++++++++++++++++++++++++++++---- 1 file changed, 184 insertions(+), 21 deletions(-) diff --git a/tests/test_action_cancel.py b/tests/test_action_cancel.py index 358fa02f..a5a4ad49 100644 --- a/tests/test_action_cancel.py +++ b/tests/test_action_cancel.py @@ -6,38 +6,201 @@ from fastapi.testclient import TestClient from temp_client import poll_task, task_href import labthings_fastapi as lt +import time -class ThingOne(lt.Thing): +class CancellableCountingThing(lt.Thing): counter = lt.ThingProperty(int, 0, observable=False) + check = lt.ThingProperty( + bool, + False, + observable=False, + description=( + "This variable is used to check that the action can detect a cancel event " + "and react by performing another task, in this case, setting this variable." + ), + ) @lt.thing_action def count_slowly(self, cancel: lt.deps.CancelHook, n: int = 10): for i in range(n): - cancel.sleep(0.1) + try: + cancel.sleep(0.1) + except lt.exceptions.InvocationCancelledError as e: + # Set check to true to show that cancel was called. + self.check = True + raise (e) self.counter += 1 + @lt.thing_action + def count_slowly_but_ignore_cancel(self, cancel: lt.deps.CancelHook, n: int = 10): + """ + Used to check that cancellation alter task behaviour + """ + counting_increment = 1 + for i in range(n): + try: + cancel.sleep(0.1) + except lt.exceptions.InvocationCancelledError: + # Rather than cancel, this disobedient task just counts faster + counting_increment = 3 + self.counter += counting_increment + + @lt.thing_action + def count_and_only_cancel_if_asked_twice( + self, cancel: lt.deps.CancelHook, n: int = 10 + ): + """ + A task that changes behaviour on cancel, but if asked a second time will cancel + """ + cancelled_once = False + counting_increment = 1 + for i in range(n): + try: + cancel.sleep(0.1) + except lt.exceptions.InvocationCancelledError as e: + # If this is the second time, this is called actually cancel. + if cancelled_once: + raise (e) + # If not, remember that this cancel event happened. + cancelled_once = True + # Reset the CancelHook + cancel.clear() + # Count backwards instead! + counting_increment = -1 + self.counter += counting_increment + def test_invocation_cancel(): + """ + Test that an invocation can be cancelled and the associated + exception handled correctly. + """ + server = lt.ThingServer() + counting_thing = CancellableCountingThing() + server.add_thing(counting_thing, "/counting_thing") + with TestClient(server.app) as client: + assert counting_thing.counter == 0 + assert not counting_thing.check + response = client.post("/counting_thing/count_slowly", json={}) + response.raise_for_status() + # Use `client.delete` to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + invocation = poll_task(client, response.json()) + assert invocation["status"] == "cancelled" + assert counting_thing.counter < 9 + # Check that error handling worked + assert counting_thing.check + + +def test_invocation_that_refuses_to_cancel(): + """ + Test that an invocation can detect a cancel request but choose + to modify behaviour. + """ + server = lt.ThingServer() + counting_thing = CancellableCountingThing() + server.add_thing(counting_thing, "/counting_thing") + with TestClient(server.app) as client: + assert counting_thing.counter == 0 + response = client.post( + "/counting_thing/count_slowly_but_ignore_cancel", json={"n": 5} + ) + response.raise_for_status() + # Use `client.delete` to try to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + invocation = poll_task(client, response.json()) + # As the task ignored the cancel. It should return completed + assert invocation["status"] == "completed" + # Counter should be greater than 5 as it counts faster if cancelled! + assert counting_thing.counter > 5 + + +def test_invocation_that_needs_cancel_twice(): + """ + Test that an invocation can interpret cancel to change behaviour, but + can really cancel if requested a second time + """ server = lt.ThingServer() - thing_one = ThingOne() - server.add_thing(thing_one, "/thing_one") + counting_thing = CancellableCountingThing() + server.add_thing(counting_thing, "/counting_thing") with TestClient(server.app) as client: - r = client.post("/thing_one/count_slowly", json={}) - r.raise_for_status() - dr = client.delete(task_href(r.json())) - dr.raise_for_status() - invocation = poll_task(client, r.json()) + # First cancel only once: + assert counting_thing.counter == 0 + response = client.post( + "/counting_thing/count_and_only_cancel_if_asked_twice", json={"n": 5} + ) + response.raise_for_status() + # Use `client.delete` to try to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + invocation = poll_task(client, response.json()) + # As the task ignored the cancel. It should return completed + assert invocation["status"] == "completed" + # Counter should be less than 0 as it should started counting backwards + # almost immediately. + assert counting_thing.counter < 0 + + # Next cancel twice. + counting_thing.counter = 0 + assert counting_thing.counter == 0 + response = client.post( + "/counting_thing/count_and_only_cancel_if_asked_twice", json={"n": 5} + ) + response.raise_for_status() + # Use `client.delete` to try to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + # Cancel again + cancel_response2 = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response2.raise_for_status() + invocation = poll_task(client, response.json()) + # As the task ignored the cancel. It should return completed assert invocation["status"] == "cancelled" - assert thing_one.counter < 9 - - # Try again, but cancel too late - should get a 503. - thing_one.counter = 0 - r = client.post("/thing_one/count_slowly", json={"n": 0}) - r.raise_for_status() - invocation = poll_task(client, r.json()) - dr = client.delete(task_href(r.json())) - assert dr.status_code == 503 - - dr = client.delete(f"/invocations/{uuid.uuid4()}") - assert dr.status_code == 404 + # Counter should be less than 0 as it should started counting backwards + # almost immediately. + assert counting_thing.counter < 0 + + +def test_late_invocation_cancel_responds_503(): + """ + Test that cancelling an invocation after it completes returns a 503 response. + """ + server = lt.ThingServer() + counting_thing = CancellableCountingThing() + server.add_thing(counting_thing, "/counting_thing") + with TestClient(server.app) as client: + assert counting_thing.counter == 0 + assert not counting_thing.check + response = client.post("/counting_thing/count_slowly", json={"n": 1}) + response.raise_for_status() + # Sleep long enough that task completes. + time.sleep(0.3) + poll_task(client, response.json()) + # Use `client.delete` to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Check a 503 code is returned + assert cancel_response.status_code == 503 + # Check counter reached it's target + assert counting_thing.counter == 1 + # Check that error handling wasn't called + assert not counting_thing.check + + +def test_cancel_unknown_task(): + """ + Test that cancelling an unknown invocation returns a 404 response + """ + server = lt.ThingServer() + counting_thing = CancellableCountingThing() + server.add_thing(counting_thing, "/counting_thing") + with TestClient(server.app) as client: + cancel_response = client.delete(f"/invocations/{uuid.uuid4()}") + assert cancel_response.status_code == 404