Fix catch (OperationCancelledException) blocks to add when guards…

… - only ignore cancellations set by caller
Particular · Feb 6, 2025 · 9c249c9 · 9c249c9
1 parent 3fbb3e8
commit 9c249c9
Show file tree

Hide file tree

Showing 28 changed files with 45 additions and 41 deletions.
diff --git a/src/Particular.LicensingComponent/AuditThroughput/AuditThroughputCollectorHostedService.cs b/src/Particular.LicensingComponent/AuditThroughput/AuditThroughputCollectorHostedService.cs
@@ -40,7 +40,7 @@ protected override async Task ExecuteAsync(CancellationToken cancellationToken)
                 }
             } while (await timer.WaitForNextTickAsync(cancellationToken));
         }
-        catch (OperationCanceledException)
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
         {
             logger.LogInformation($"Stopping {nameof(AuditThroughputCollectorHostedService)}");
         }

diff --git a/src/Particular.LicensingComponent/BrokerThroughput/BrokerThroughputCollectorHostedService.cs b/src/Particular.LicensingComponent/BrokerThroughput/BrokerThroughputCollectorHostedService.cs
@@ -53,7 +53,7 @@ static ReadOnlyDictionary<string, string> LoadBrokerSettingValues(IEnumerable<Ke
                 }
             } while (await timer.WaitForNextTickAsync(stoppingToken));
         }
-        catch (OperationCanceledException)
+        catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
         {
             logger.LogInformation($"Stopping {nameof(BrokerThroughputCollectorHostedService)}");
         }

diff --git a/src/ServiceControl.AcceptanceTesting/ScenarioWithEndpointBehaviorExtensions.cs b/src/ServiceControl.AcceptanceTesting/ScenarioWithEndpointBehaviorExtensions.cs
@@ -139,9 +139,9 @@ public override async Task Stop(CancellationToken cancellationToken = default)
                 {
                     await checkTask;
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
                 {
-                    //Swallow
+                    // Even though we are stopping, ONLY swallow when OCE from callee to not hide any ungraceful stop errors
                 }
                 finally
                 {

diff --git a/src/ServiceControl.Audit/Auditing/ImportFailedAudits.cs b/src/ServiceControl.Audit/Auditing/ImportFailedAudits.cs
@@ -48,7 +48,7 @@ await failedAuditStore.ProcessFailedMessages(
                                 Logger.Debug($"Successfully re-imported failed audit message {transportMessage.Id}.");
                             }
                         }
-                        catch (OperationCanceledException)
+                        catch (OperationCanceledException) when (token.IsCancellationRequested)
                         {
                             // no-op
                         }

diff --git a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/ImportFailedAuditsCommand.cs b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/ImportFailedAuditsCommand.cs
@@ -37,7 +37,7 @@ public override async Task Execute(HostArguments args, Settings settings)
             {
                 await importer.Run(tokenSource.Token);
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (tokenSource.IsCancellationRequested)
             {
                 // no op
             }

diff --git a/src/ServiceControl.Infrastructure.Metrics/MetricsReporter.cs b/src/ServiceControl.Infrastructure.Metrics/MetricsReporter.cs
@@ -32,7 +32,7 @@ public void Start()
                         await Task.Delay(interval, tokenSource.Token).ConfigureAwait(false);
                     }
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (tokenSource.IsCancellationRequested)
                 {
                     //no-op
                 }

diff --git a/src/ServiceControl.Infrastructure/AsyncTimer.cs b/src/ServiceControl.Infrastructure/AsyncTimer.cs
@@ -40,7 +40,7 @@ public TimerJob(Func<CancellationToken, Task<TimerJobExecutionResult>> callback,
 
                             //Otherwise execute immediately
                         }
-                        catch (OperationCanceledException)
+                        catch (OperationCanceledException) when (token.IsCancellationRequested)
                         {
                             // no-op
                         }
@@ -50,7 +50,7 @@ public TimerJob(Func<CancellationToken, Task<TimerJobExecutionResult>> callback,
                         }
                     }
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (token.IsCancellationRequested)
                 {
                     // no-op
                 }
@@ -64,7 +64,7 @@ public async Task Stop()
                 return;
             }
 
-            tokenSource.Cancel();
+            await tokenSource.CancelAsync().ConfigureAwait(false);
             tokenSource.Dispose();
 
             if (task != null)
@@ -73,7 +73,7 @@ public async Task Stop()
                 {
                     await task.ConfigureAwait(false);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (tokenSource.IsCancellationRequested)
                 {
                     //NOOP
                 }

diff --git a/src/ServiceControl.Infrastructure/ReadOnlyStream.cs b/src/ServiceControl.Infrastructure/ReadOnlyStream.cs
@@ -51,7 +51,7 @@ public override Task CopyToAsync(Stream destination, int bufferSize, Cancellatio
 
             return Task.CompletedTask;
         }
-        catch (OperationCanceledException e)
+        catch (OperationCanceledException e) when (cancellationToken.IsCancellationRequested)
         {
             return Task.FromCanceled(e.CancellationToken);
         }
@@ -113,7 +113,7 @@ public override Task<int> ReadAsync(byte[] buffer, int offset, int count, Cancel
 
             return Task.FromResult(result);
         }
-        catch (OperationCanceledException e)
+        catch (OperationCanceledException e) when (cancellationToken.IsCancellationRequested)
         {
             return Task.FromCanceled<int>(e.CancellationToken);
         }
@@ -136,7 +136,7 @@ public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken
 
             return new ValueTask<int>(result);
         }
-        catch (OperationCanceledException e)
+        catch (OperationCanceledException e) when (cancellationToken.IsCancellationRequested)
         {
             return new ValueTask<int>(Task.FromCanceled<int>(e.CancellationToken));
         }

diff --git a/src/ServiceControl.Infrastructure/Watchdog.cs b/src/ServiceControl.Infrastructure/Watchdog.cs
@@ -54,9 +54,9 @@ public Task Start(Action onFailedOnStartup)
 
                         failedOnStartup ??= false;
                     }
-                    catch (OperationCanceledException)
+                    catch (OperationCanceledException) when (!shutdownTokenSource.IsCancellationRequested)
                     {
-                        //Do not Delay
+                        // Continue, as OCE is not from caller
                         continue;
                     }
                     catch (Exception e)
@@ -81,7 +81,7 @@ public Task Start(Action onFailedOnStartup)
                     {
                         await Task.Delay(timeToWaitBetweenStartupAttempts, shutdownTokenSource.Token).ConfigureAwait(false);
                     }
-                    catch (OperationCanceledException)
+                    catch (OperationCanceledException) when (shutdownTokenSource.IsCancellationRequested)
                     {
                         //Ignore
                     }

diff --git a/src/ServiceControl.Monitoring/Infrastructure/RemoveExpiredEndpointInstances.cs b/src/ServiceControl.Monitoring/Infrastructure/RemoveExpiredEndpointInstances.cs
@@ -48,7 +48,7 @@ protected override async Task ExecuteAsync(CancellationToken cancellationToken)
                 }
             } while (await timer.WaitForNextTickAsync(cancellationToken));
         }
-        catch (OperationCanceledException)
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
         {
             logger.LogInformation($"Stopping {nameof(RemoveExpiredEndpointInstances)} timer");
         }

diff --git a/src/ServiceControl.Monitoring/Infrastructure/ReportThroughputHostedService.cs b/src/ServiceControl.Monitoring/Infrastructure/ReportThroughputHostedService.cs
@@ -42,7 +42,7 @@ protected override async Task ExecuteAsync(CancellationToken cancellationToken)
                     }
                 } while (await timer.WaitForNextTickAsync(cancellationToken));
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
                 logger.LogInformation($"Stopping {nameof(ReportThroughputHostedService)} timer");
             }

diff --git a/src/ServiceControl.Persistence.RavenDB/ExternalIntegrationRequestsDataStore.cs b/src/ServiceControl.Persistence.RavenDB/ExternalIntegrationRequestsDataStore.cs
@@ -82,7 +82,7 @@ async Task StartDispatcherTask(CancellationToken cancellationToken)
                         await signal.WaitHandle.WaitOneAsync(cancellationToken);
                         signal.Reset();
                     }
-                    catch (OperationCanceledException)
+                    catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
                     {
                         break;
                     }
@@ -91,7 +91,7 @@ async Task StartDispatcherTask(CancellationToken cancellationToken)
                 }
                 while (!cancellationToken.IsCancellationRequested);
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
                 // ignore
             }

diff --git a/src/ServiceControl.Persistence.RavenDB/FailedErrorImportDataStore.cs b/src/ServiceControl.Persistence.RavenDB/FailedErrorImportDataStore.cs
@@ -35,7 +35,7 @@ public async Task ProcessFailedErrorImports(Func<FailedTransportMessage, Task> p
                             Logger.Debug($"Successfully re-imported failed error message {transportMessage.Id}.");
                         }
                     }
-                    catch (OperationCanceledException)
+                    catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
                     {
                         //  no-op
                     }

diff --git a/src/ServiceControl.RavenDB/EmbeddedDatabase.cs b/src/ServiceControl.RavenDB/EmbeddedDatabase.cs
@@ -110,7 +110,7 @@ void Start(ServerOptions serverOptions)
 
                         Logger.Info("RavenDB server process restarted successfully.");
                     }
-                    catch (OperationCanceledException)
+                    catch (OperationCanceledException) when (shutdownCancellationToken.IsCancellationRequested)
                     {
                         //no-op
                     }

diff --git a/src/ServiceControl.Transports.ASBS/QueueLengthProvider.cs b/src/ServiceControl.Transports.ASBS/QueueLengthProvider.cs
@@ -43,7 +43,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                     UpdateAllQueueLengths(queueRuntimeInfos);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
                 {
                     // no-op
                 }

diff --git a/src/ServiceControl.Transports.ASQ/QueueLengthProvider.cs b/src/ServiceControl.Transports.ASQ/QueueLengthProvider.cs
@@ -42,7 +42,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                     await Task.Delay(QueryDelayInterval, stoppingToken);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
                 {
                     // no-op
                 }
@@ -67,7 +67,7 @@ async Task FetchLength(QueueLengthValue queueLength, CancellationToken cancellat
 
                 problematicQueuesNames.TryRemove(queueLength.QueueName, out _);
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
                 // no-op
             }

diff --git a/src/ServiceControl.Transports.Learning/QueueLengthProvider.cs b/src/ServiceControl.Transports.Learning/QueueLengthProvider.cs
@@ -30,7 +30,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                     await Task.Delay(QueryDelayInterval, stoppingToken);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
                 {
                     // It's OK. We're shutting down
                 }

diff --git a/src/ServiceControl.Transports.PostgreSql/QueueLengthProvider.cs b/src/ServiceControl.Transports.PostgreSql/QueueLengthProvider.cs
@@ -49,7 +49,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                 UpdateQueueLengthStore();
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
             {
                 // no-op
             }

diff --git a/src/ServiceControl.Transports.RabbitMQ/QueueLengthProvider.cs b/src/ServiceControl.Transports.RabbitMQ/QueueLengthProvider.cs
@@ -39,7 +39,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                     await Task.Delay(QueryDelayInterval, stoppingToken);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
                 {
                     // no-op
                 }
@@ -141,7 +141,7 @@ public async Task Execute(Action<IModel> action, CancellationToken cancellationT
 
                     action(model);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
                 {
                     // no-op
                 }

diff --git a/src/ServiceControl.Transports.SQS/QueueLengthProvider.cs b/src/ServiceControl.Transports.SQS/QueueLengthProvider.cs
@@ -65,7 +65,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                     await Task.Delay(QueryDelayInterval, stoppingToken);
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
                 {
                     // no-op
                 }
@@ -104,7 +104,7 @@ async Task FetchLength(string queue, IAmazonSQS client, QueueAttributesRequestCa
                 var response = await client.GetQueueAttributesAsync(attReq, cancellationToken);
                 sizes[queue] = response.ApproximateNumberOfMessages;
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
                 // no-op
             }

diff --git a/src/ServiceControl.Transports.SqlServer/QueueLengthProvider.cs b/src/ServiceControl.Transports.SqlServer/QueueLengthProvider.cs
@@ -47,7 +47,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 
                     UpdateQueueLengthStore();
                 }
-                catch (OperationCanceledException)
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
                 {
                     // no-op
                 }

diff --git a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs
@@ -125,7 +125,7 @@ async Task<QueryResult<TOut>> FetchAndParse(HttpClient httpClient, string pathAn
                     httpRequestException);
                 return QueryResult<TOut>.Empty();
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) // Intentional, used to gracefully handle timeout
             {
                 logger.Warn($"Failed to query remote instance at {remoteInstanceSetting.BaseAddress} due to a timeout");
                 return QueryResult<TOut>.Empty();

diff --git a/src/ServiceControl/CustomChecks/InternalCustomChecks/InternalCustomCheckManager.cs b/src/ServiceControl/CustomChecks/InternalCustomChecks/InternalCustomCheckManager.cs
@@ -40,7 +40,7 @@ async Task<TimerJobExecutionResult> Run(CancellationToken cancellationToken)
             {
                 result = await check.PerformCheck(cancellationToken);
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
                 //Do nothing as we are shutting down
             }

diff --git a/src/ServiceControl/Hosting/Commands/ImportFailedErrorsCommand.cs b/src/ServiceControl/Hosting/Commands/ImportFailedErrorsCommand.cs
@@ -38,7 +38,7 @@ public override async Task Execute(HostArguments args, Settings settings)
             {
                 await importFailedErrors.Run(tokenSource.Token);
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (tokenSource.IsCancellationRequested)
             {
                 // no-op
             }

diff --git a/src/ServiceControl/Infrastructure/Metrics/MetricsReporterHostedService.cs b/src/ServiceControl/Infrastructure/Metrics/MetricsReporterHostedService.cs
@@ -30,7 +30,7 @@ public async Task StopAsync(CancellationToken cancellationToken)
             {
                 await reporter.Stop();
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
                 //NOOP
             }

diff --git a/src/ServiceControl/Monitoring/HeartbeatEndpointSettingsSyncHostedService.cs b/src/ServiceControl/Monitoring/HeartbeatEndpointSettingsSyncHostedService.cs
@@ -45,7 +45,7 @@ protected override async Task ExecuteAsync(CancellationToken cancellationToken)
                 }
             } while (await timer.WaitForNextTickAsync(cancellationToken));
         }
-        catch (OperationCanceledException)
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
         {
             logger.LogInformation($"Stopping {nameof(HeartbeatEndpointSettingsSyncHostedService)}");
         }

diff --git a/src/ServiceControl/Monitoring/HeartbeatMonitoringHostedService.cs b/src/ServiceControl/Monitoring/HeartbeatMonitoringHostedService.cs
@@ -30,9 +30,9 @@ public async Task StopAsync(CancellationToken cancellationToken)
             {
                 await timer.Stop();
             }
-            catch (OperationCanceledException)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
             {
-                //NOOP
+                //NOOP, invoked Stop does not
             }
         }
 

diff --git a/src/ServiceControl/Operations/CheckRemotes.cs b/src/ServiceControl/Operations/CheckRemotes.cs
@@ -73,7 +73,11 @@ static async Task CheckSuccessStatusCode(IHttpClientFactory httpClientFactory, R
                 remoteSettings.TemporarilyUnavailable = true;
                 throw new TimeoutException($"The remote instance at '{remoteSettings.BaseAddress}' doesn't seem to be available. It will be temporarily disabled. Reason: {e.Message}", e);
             }
-            catch (OperationCanceledException e)
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+            {
+                // Cancelled, noop
+            }
+            catch (OperationCanceledException e) // Intentional, OCE gracefully handled by other catch
             {
                 remoteSettings.TemporarilyUnavailable = true;
                 throw new TimeoutException($"The remote at '{remoteSettings.BaseAddress}' did not respond within the allotted time of '{queryTimeout}'. It will be temporarily disabled.", e);
-Original file line number
+Diff line change
@@ Expand Up / @@ -32,7 +32,7 @@ public void Start() @@
                             await Task.Delay(interval, tokenSource.Token).ConfigureAwait(false);
                         }
                     }
-                    catch (OperationCanceledException)
+                    catch (OperationCanceledException) when (tokenSource.IsCancellationRequested)
                     {
                         //no-op
                     }
@@ Expand Down @@