[Feature Request] temporalio.CancelledError should inherit from BaseException

Abierto
#1,292 8 comentarios 1 reacción 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
48/100
Tipo de issue
Nueva funcionalidad
Claridad
Bastante claro
Estado de actividad
Tranquilo
Stack tecnológico
python
Área
backend

Línea de trabajo

Empieza en temporalio/exceptions.py, alrededor de la definición de CancelledError en la línea 145, y después revisa cómo se lanza y se gestiona la cancelación en todo el SDK. Confirma el impacto en los manejadores existentes de Exception y define el comportamiento de compatibilidad requerido antes de cambiar la herencia; se considera terminado cuando la cancelación no es capturada inadvertidamente por manejadores generales de Exception y el comportamiento relevante está cubierto por pruebas.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

enhancement
Cancelled Error Deriving from Exception makes it easy to inadvertently ignore Cancellation Attempts

Currently temporalio.CancelledError inherits from temporalio.FailureError which in turn inherits from Exception.

I would like to suggest that the semantics of this type of error would be more appropriate if it derived from BaseException, primarily because of expectations and patterns of how existing application code might be handling those various cases.

Basically it is very common in user/app code to have fairly broad exception cases like except Exception as e: etc. This is arguably not "best practice" (you should really catch more specific individual exception types), but it happens a lot.

My understanding is that Temporal Cancellation errors are implemented via a fairly low-level Python C-extension API, which allows them to forcibly preempt arbitrary user code. This means those cancellation errors can surface at arbitrary points in the call-graph, i.e. they pop up in random places in User code, not just in the "outer layer" of Temporal workflow code.

This creates a scenario where it's easy for error handling logic in random application code to unintentionally ignore Temporal Cancellation attempts, which leads to lots of confusion when workflows that should have been cancelled keep running.

I believe this type of semantics is why the core Python runtime typically uses BaseException for things like hard system interrupts, cancellation errors, OOMs, etc. And you can actually see that the stdlib changed the base class of asyncio.CancelledError
from Exception to BaseException, I believe for similar reasons.

I realize that while this change is simple to implement it would have meaningful consequences for existing user code, but I wanted to propose the idea and see if this is something that has come up in discussion before, or if the community has any other suggestions on how to handle temporal CancelledError when working with existing code that may be handling Exception broadly.

Lenguaje dominante
Python
Estrellas
1.2k
Forks
241
Merge medio
3 d 2 h
PR fusionados (30 d)
49

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de temporalio/sdk-python

Todos los issues de temporalio/sdk-python

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.