Skip to content

Fixed the WorkflowInstanceHandler to handle exceptions thrown while persisting log batches, which were crashing the operator - #565

Open
RaphaelFakhri wants to merge 1 commit into
serverlessworkflow:mainfrom
RaphaelFakhri:fix-564-log-batch-crash
Open

RaphaelFakhri wants to merge 1 commit into
serverlessworkflow:mainfrom
RaphaelFakhri:fix-564-log-batch-crash

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Fixes #564.

WorkflowInstanceHandler persists runner logs from a System.Threading.Timer whose callback is an async void lambda. OnPersistLogBatchAsync had no exception handling, so any exception escaped the callback and terminated the operator process:

  • A tick that runs while the handler is being disposed reads CancellationTokenSource.Token after disposal and throws ObjectDisposedException.
  • A failure of ITextDocumentRepository.AppendAsync, for example a transient Redis error, also crashes the process.

This PR changes OnPersistLogBatchAsync to:

  • Return early once the handler is disposed.
  • Ignore ObjectDisposedException and OperationCanceledException, which only occur while the handler shuts down.
  • Log any other exception instead of letting it escape the timer callback.
  • Reset _persistingLogs in a finally block so the next tick persists the next batch.

Special notes for reviewers:

  • The new WorkflowInstanceHandlerTests require a reference from Synapse.UnitTests to Synapse.Operator. That reference brings the Synapse.Operator namespace into scope, so MockSynapseApiClient now uses Synapse.Resources.Operator instead of Operator.
  • Both new tests fail on main (ObjectDisposedException and IOException) and pass with this change.

Additional information (if needed):

A batch whose AppendAsync call fails is logged and dropped. The operator keeps running and the next batch is persisted.

… persisting log batches, which were crashing the operator

Fixes serverlessworkflow#564

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Operator process crashes with ObjectDisposedException in WorkflowInstanceHandler.OnPersistLogBatchAsync when a handler is disposed

1 participant