Return HTTP 400 for disabled features (#5527)

dotnet · Oct 13, 2023 · bb6e92b · bb6e92b
1 parent ca1f597
commit bb6e92b
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 45 deletions.
diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/ControllerExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/ControllerExtensions.cs
@@ -17,6 +17,15 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi.Controllers
 {
     internal static class ControllerExtensions
     {
+        public static ActionResult FeatureNotEnabled(this ControllerBase controller, string featureName)
+        {
+            return new BadRequestObjectResult(new ProblemDetails()
+            {
+                Detail = string.Format(Strings.Message_FeatureNotEnabled, featureName),
+                Status = StatusCodes.Status400BadRequest
+            });
+        }
+
         public static ActionResult NotAcceptable(this ControllerBase controller)
         {
             return new StatusCodeResult((int)HttpStatusCode.NotAcceptable);

diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs
@@ -584,7 +584,7 @@ public async Task<ActionResult> CaptureParameters(
         {
             if (!_parameterCapturingOptions.Value.GetEnabled())
             {
-                return NotFound();
+                return this.FeatureNotEnabled(Strings.FeatureName_ParameterCapturing);
             }
 
             ProcessKey? processKey = Utilities.GetProcessKey(pid, uid, name);
@@ -617,7 +617,7 @@ public Task<ActionResult> CaptureStacks(
         {
             if (!_callStacksOptions.Value.GetEnabled())
             {
-                return Task.FromResult<ActionResult>(NotFound());
+                return Task.FromResult<ActionResult>(this.FeatureNotEnabled(Strings.FeatureName_CallStacks));
             }
 
             ProcessKey? processKey = Utilities.GetProcessKey(pid, uid, name);

diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/ExceptionsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/ExceptionsController.cs
@@ -6,7 +6,6 @@
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Diagnostics.Monitoring.Options;
 using Microsoft.Diagnostics.Monitoring.WebApi.Exceptions;
-using Microsoft.Diagnostics.Monitoring.WebApi.Models;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -64,7 +63,7 @@ public Task<ActionResult> GetExceptions(
         {
             if (!_options.Value.GetEnabled())
             {
-                return Task.FromResult<ActionResult>(NotFound());
+                return Task.FromResult<ActionResult>(this.FeatureNotEnabled(Strings.FeatureName_Exceptions));
             }
 
             ProcessKey? processKey = Utilities.GetProcessKey(pid, uid, name);

diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs
diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx
@@ -213,6 +213,15 @@
     <value>Value must be of type string.</value>
     <comment>Gets a string similar to "Value must be of type string.".</comment>
   </data>
+  <data name="FeatureName_CallStacks" xml:space="preserve">
+    <value>Call Stacks</value>
+  </data>
+  <data name="FeatureName_Exceptions" xml:space="preserve">
+    <value>Exceptions</value>
+  </data>
+  <data name="FeatureName_ParameterCapturing" xml:space="preserve">
+    <value>Parameter Capturing</value>
+  </data>
   <data name="LogFormatString_CounterEndedPayload" xml:space="preserve">
     <value>The counter {0} ended and is no longer receiving metrics.</value>
   </data>
@@ -314,6 +323,9 @@
   <data name="Message_CollectionRuleStateReason_Throttled" xml:space="preserve">
     <value>This collection rule is temporarily throttled because the ActionCountLimit has been reached within the ActionCountSlidingWindowDuration.</value>
   </data>
+  <data name="Message_FeatureNotEnabled" xml:space="preserve">
+    <value>The '{0}' feature is not enabled.</value>
+  </data>
   <data name="Message_GeneratedInProcessArtifact" xml:space="preserve">
     <value>Finished generating in-process artifact</value>
   </data>

diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/StacksTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/StacksTests.cs
@@ -247,8 +247,8 @@ public Task TestInProcessFeaturesEnabledCallStacksDisabled(Architecture targetAr
                 {
                     int processId = await runner.ProcessIdTask;
 
-                    ApiStatusCodeException ex = await Assert.ThrowsAsync<ApiStatusCodeException>(() => client.CaptureStacksAsync(processId, WebApi.StackFormat.Json));
-                    Assert.Equal(HttpStatusCode.NotFound, ex.StatusCode);
+                    ValidationProblemDetailsException ex = await Assert.ThrowsAsync<ValidationProblemDetailsException>(() => client.CaptureStacksAsync(processId, WebApi.StackFormat.Json));
+                    Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode);
 
                     await runner.SendCommandAsync(TestAppScenarios.Stacks.Commands.Continue);
                 },
@@ -279,8 +279,8 @@ public Task TestInProcessFeaturesDisabledCallStacksEnabled(Architecture targetAr
                 {
                     int processId = await runner.ProcessIdTask;
 
-                    ApiStatusCodeException ex = await Assert.ThrowsAsync<ApiStatusCodeException>(() => client.CaptureStacksAsync(processId, WebApi.StackFormat.Json));
-                    Assert.Equal(HttpStatusCode.NotFound, ex.StatusCode);
+                    ValidationProblemDetailsException ex = await Assert.ThrowsAsync<ValidationProblemDetailsException>(() => client.CaptureStacksAsync(processId, WebApi.StackFormat.Json));
+                    Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode);
 
                     await runner.SendCommandAsync(TestAppScenarios.Stacks.Commands.Continue);
                 },

diff --git a/src/Tools/dotnet-monitor/Experimental/ExperimentalStartupLogger.cs b/src/Tools/dotnet-monitor/Experimental/ExperimentalStartupLogger.cs
@@ -23,7 +23,7 @@ public void Log()
         {
             if (_parameterCapturingOptions.GetEnabled())
             {
-                _logger.ExperimentalFeatureEnabled(Strings.FeatureName_ParameterCapturing);
+                _logger.ExperimentalFeatureEnabled(Microsoft.Diagnostics.Monitoring.WebApi.Strings.FeatureName_ParameterCapturing);
             }
 
             // Experimental features should log a warning when they are activated e.g.

diff --git a/src/Tools/dotnet-monitor/Strings.Designer.cs b/src/Tools/dotnet-monitor/Strings.Designer.cs
diff --git a/src/Tools/dotnet-monitor/Strings.resx b/src/Tools/dotnet-monitor/Strings.resx
@@ -377,12 +377,6 @@
     <value>'{0}' is not a known trigger type.</value>
     <comment>{0} = The type of trigger that is unknown.</comment>
   </data>
-  <data name="FeatureName_CallStacks" xml:space="preserve">
-    <value>Call Stacks</value>
-  </data>
-  <data name="FeatureName_Exceptions" xml:space="preserve">
-    <value>Exceptions</value>
-  </data>
   <data name="HelpDescription_CommandCollect" xml:space="preserve">
     <value>Monitor logs and metrics in a .NET application send the results to a chosen destination.</value>
     <comment>Gets the string to display in help that explains what the 'collect' command does.</comment>
@@ -913,9 +907,6 @@
 0. providerName: The name of the provider that failed validation.
 1. errorMessage: The validation failure message</comment>
   </data>
-  <data name="FeatureName_ParameterCapturing" xml:space="preserve">
-    <value>Parameter Capturing</value>
-  </data>
   <data name="ErrorMessage_UnableToFindHostingStartupAssembly" xml:space="preserve">
     <value>Unable to find hosting startup assembly at determined path.</value>
   </data>