Skip to content

Commit ab93270

Browse files
Revert usage of asyncio cancellation mechanism (#142)
* Revert usage of asyncio cancellation mechanism (#132) * Improvements to exception, expose SdkInternalBaseException and is_internal_exception
1 parent 8ba5587 commit ab93270

File tree

4 files changed

+65
-30
lines changed

4 files changed

+65
-30
lines changed

python/restate/__init__.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from .retry_policy import InvocationRetryPolicy
2323
# pylint: disable=line-too-long
2424
from .context import DurablePromise, RestateDurableFuture, RestateDurableCallFuture, RestateDurableSleepFuture, SendHandle, RunOptions
25-
from .exceptions import TerminalError
25+
from .exceptions import TerminalError, SdkInternalBaseException, is_internal_exception
2626
from .asyncio import as_completed, gather, wait_completed, select
2727

2828
from .endpoint import app
@@ -67,5 +67,7 @@ def test_harness(app, # type: ignore
6767
"select",
6868
"logging",
6969
"RestateLoggingFilter",
70-
"InvocationRetryPolicy"
70+
"InvocationRetryPolicy",
71+
"SdkInternalBaseException",
72+
"is_internal_exception"
7173
]

python/restate/exceptions.py

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,53 @@
1010
#
1111
"""This module contains the restate exceptions"""
1212

13-
class TerminalError(Exception):
14-
"""This exception is raised to indicate a termination of the execution"""
13+
# pylint: disable=C0301
1514

15+
class TerminalError(Exception):
16+
"""This exception is thrown to indicate that Restate should not retry."""
1617
def __init__(self, message: str, status_code: int = 500) -> None:
1718
super().__init__(message)
1819
self.message = message
1920
self.status_code = status_code
21+
22+
23+
class SdkInternalBaseException(Exception):
24+
"""This exception is internal, and you should not catch it.
25+
If you need to distinguish with other exceptions, use is_internal_exception."""
26+
def __init__(self, message: str) -> None:
27+
super().__init__(
28+
message +
29+
"""
30+
This exception is safe to ignore. If you see it, you might be using a try/catch all statement.
31+
32+
Don't do:
33+
try:
34+
# Code
35+
except:
36+
# This catches all exceptions, including the SdkInternalBaseException!
37+
38+
Do instead:
39+
try:
40+
# Code
41+
except TerminalError:
42+
# In Restate handlers you typically want to catch TerminalError only
43+
44+
Or remove the try/except altogether if you don't need it.
45+
For further info on error handling, refer to https://docs.restate.dev/develop/python/error-handling
46+
""")
47+
48+
class SuspendedException(SdkInternalBaseException):
49+
"""This exception is raised to indicate that the execution is suspended"""
50+
def __init__(self) -> None:
51+
super().__init__("Invocation got suspended, Restate will resume this invocation when progress can be made.")
52+
53+
class SdkInternalException(SdkInternalBaseException):
54+
"""This exception is raised to indicate that the execution raised a retryable error."""
55+
def __init__(self) -> None:
56+
super().__init__("Invocation attempt raised a retryable error.\n"
57+
"Restate will retry executing this invocation from the point where it left off.")
58+
59+
60+
def is_internal_exception(e) -> bool:
61+
"""Returns true if the exception is an internal Restate exception"""
62+
return isinstance(e, SdkInternalBaseException)

python/restate/server_context.py

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import time
3131

3232
from restate.context import DurablePromise, AttemptFinishedEvent, HandlerType, ObjectContext, Request, RestateDurableCallFuture, RestateDurableFuture, RunAction, SendHandle, RestateDurableSleepFuture, RunOptions, P
33-
from restate.exceptions import TerminalError
33+
from restate.exceptions import TerminalError, SdkInternalBaseException, SdkInternalException, SuspendedException
3434
from restate.handler import Handler, handler_from_callable, invoke_handler
3535
from restate.serde import BytesSerde, DefaultSerde, JsonSerde, Serde
3636
from restate.server_types import ReceiveChannel, Send
@@ -273,19 +273,6 @@ def update_restate_context_is_replaying(vm: VMWrapper):
273273
"""Update the context var 'restate_context_is_replaying'. This should be called after each vm.sys_*"""
274274
restate_context_is_replaying.set(vm.is_replaying())
275275

276-
async def cancel_current_task():
277-
"""Cancel the current task"""
278-
current_task = asyncio.current_task()
279-
if current_task is not None:
280-
# Cancel through asyncio API
281-
current_task.cancel(
282-
"Cancelled by Restate SDK, you should not call any Context method after this exception is thrown."
283-
)
284-
# Sleep 0 will pop up the cancellation
285-
await asyncio.sleep(0)
286-
else:
287-
raise asyncio.CancelledError("Cancelled by Restate SDK, you should not call any Context method after this exception is thrown.")
288-
289276
# pylint: disable=R0902
290277
class ServerInvocationContext(ObjectContext):
291278
"""This class implements the context for the restate framework based on the server."""
@@ -327,6 +314,8 @@ async def enter(self):
327314
# pylint: disable=W0718
328315
except asyncio.CancelledError:
329316
pass
317+
except SdkInternalBaseException:
318+
pass
330319
except DisconnectedException:
331320
raise
332321
except Exception as e:
@@ -393,11 +382,11 @@ async def must_take_notification(self, handle):
393382
await self.take_and_send_output()
394383
# Print this exception, might be relevant for the user
395384
traceback.print_exception(res)
396-
await cancel_current_task()
385+
raise SdkInternalException() from res
397386
if isinstance(res, Suspended):
398387
# We might need to write out something at this point.
399388
await self.take_and_send_output()
400-
await cancel_current_task()
389+
raise SuspendedException()
401390
if isinstance(res, NotReady):
402391
raise ValueError(f"Unexpected value error: {handle}")
403392
if res is None:
@@ -414,9 +403,9 @@ async def create_poll_or_cancel_coroutine(self, handles: typing.List[int]) -> No
414403
if isinstance(do_progress_response, BaseException):
415404
# Print this exception, might be relevant for the user
416405
traceback.print_exception(do_progress_response)
417-
await cancel_current_task()
406+
raise SdkInternalException() from do_progress_response
418407
if isinstance(do_progress_response, Suspended):
419-
await cancel_current_task()
408+
raise SuspendedException()
420409
if isinstance(do_progress_response, DoProgressAnyCompleted):
421410
# One of the handles completed
422411
return
@@ -565,6 +554,8 @@ async def create_run_coroutine(self,
565554
self.vm.propose_run_completion_failure(handle, failure)
566555
except asyncio.CancelledError as e:
567556
raise e from None
557+
except SdkInternalBaseException as e:
558+
raise e from None
568559
# pylint: disable=W0718
569560
except Exception as e:
570561
end = time.time()

src/lib.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -784,23 +784,22 @@ impl ErrorFormatter for PythonErrorFormatter {
784784
fn display_closed_error(&self, f: &mut fmt::Formatter<'_>, event: &str) -> fmt::Result {
785785
write!(f, "Execution is suspended, but the handler is still attempting to make progress (calling '{event}'). This can happen:
786786
787-
* If you don't need to handle task cancellation, just avoid catch all statements. Don't do:
787+
* If you use try/catch all statements.
788+
Don't do:
788789
try:
789790
# Code
790791
except:
791-
# This catches all exceptions, including the asyncio.CancelledError!
792+
# This catches all exceptions, including the SdkInternalBaseException!
792793
# '{event}' <- This operation prints this exception
793794
794795
Do instead:
795796
try:
796797
# Code
797-
except TerminalException:
798-
# In Restate handlers you typically want to catch TerminalException only
798+
except TerminalError:
799+
# In Restate handlers you typically want to catch TerminalError only
799800
800-
* To catch ctx.run/ctx.run_typed errors, check https://docs.restate.dev/develop/python/durable-steps#run for more details.
801-
802-
* If the asyncio.CancelledError is caught, you must not run any Context operation in the except arm.
803-
Check https://docs.python.org/3/library/asyncio-task.html#task-cancellation for more details on task cancellation.
801+
Or remove the try/except altogether if you don't need it.
802+
For further info on error handling, refer to https://docs.restate.dev/develop/python/error-handling
804803
805804
* If you use the context after the handler completed, e.g. moving the context to another thread.
806805
Check https://docs.restate.dev/develop/python/concurrent-tasks for more details on how to create durable concurrent tasks in Python.")

0 commit comments

Comments
 (0)