Skip to content

Commit 531e19e

Browse files
committed
Logging controller exceptions and formatting controller error responses
1 parent a6c1767 commit 531e19e

File tree

10 files changed

+311
-2
lines changed

10 files changed

+311
-2
lines changed

src/WebJobs.Script.WebHost/App_Start/WebApiConfig.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.IO;
66
using System.Web;
77
using System.Web.Http;
8+
using System.Web.Http.ExceptionHandling;
89
using Autofac;
910
using Autofac.Integration.WebApi;
1011
using Microsoft.Azure.WebJobs.Script.Config;
@@ -49,6 +50,7 @@ public static void Register(HttpConfiguration config, ScriptSettingsManager sett
4950
var container = builder.Build();
5051
config.DependencyResolver = new AutofacWebApiDependencyResolver(container);
5152
config.Formatters.Add(new PlaintextMediaTypeFormatter());
53+
config.Services.Replace(typeof(IExceptionHandler), new ExceptionProcessingHandler(config));
5254
AddMessageHandlers(config);
5355

5456
// Web API configuration and services

src/WebJobs.Script.WebHost/GlobalSuppressions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,5 +93,11 @@
9393
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1721:PropertyNamesShouldNotMatchGetMethods", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.FunctionSecrets.#Keys")]
9494
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.HostSecrets.#SystemKeys")]
9595
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1813:AvoidUnsealedAttributes", Scope = "type", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Filters.AuthorizationLevelAttribute")]
96+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1019:DefineAccessorsForAttributeArguments", Scope = "type", Target = "Microsoft.Azure.WebJobs.Script.WebHost.ScriptExceptionFilter")]
97+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1019:DefineAccessorsForAttributeArguments", Scope = "type", Target = "Microsoft.Azure.WebJobs.Script.WebHost.ScriptExceptionFilterAttribute")]
98+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Models.ApiErrorModel.#Arguments")]
99+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "cancellationToken", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Controllers.ExceptionProcessingHandler.#HandleException(System.Net.Http.HttpRequestMessage,System.Exception,System.Threading.CancellationToken)")]
100+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Controllers.ExceptionProcessingHandler.#Handle(System.Web.Http.ExceptionHandling.ExceptionHandlerContext)")]
101+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1813:AvoidUnsealedAttributes", Scope = "type", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Filters.AuthorizationLevelAttribute")]
96102
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Models.Swagger.HttpOperationInfo.#InputParameters")]
97103
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly", Scope = "member", Target = "Microsoft.Azure.WebJobs.Script.WebHost.Models.Swagger.SwaggerDocument.#ApiEndpoints")]
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Diagnostics;
7+
using System.Net;
8+
using System.Net.Http;
9+
using System.Security.Cryptography;
10+
using System.Web.Http;
11+
using System.Web.Http.ExceptionHandling;
12+
using System.Web.Http.Results;
13+
using Microsoft.Azure.WebJobs.Host;
14+
using Microsoft.Azure.WebJobs.Host.Diagnostics;
15+
using Microsoft.Azure.WebJobs.Script.WebHost.Models;
16+
using Newtonsoft.Json;
17+
using ExceptionProcessor = System.Action<System.Web.Http.ExceptionHandling.ExceptionContext,
18+
Microsoft.Azure.WebJobs.Script.AuthorizationLevel, Microsoft.Azure.WebJobs.Script.WebHost.Models.ApiErrorModel>;
19+
20+
namespace Microsoft.Azure.WebJobs.Script.WebHost.Controllers
21+
{
22+
public class ExceptionProcessingHandler : ExceptionHandler
23+
{
24+
private readonly IDictionary<Type, ExceptionProcessor> _handlers;
25+
private readonly HttpConfiguration _config;
26+
private readonly Lazy<TraceWriter> _traceWriterLoader;
27+
28+
public ExceptionProcessingHandler(HttpConfiguration config)
29+
{
30+
if (config == null)
31+
{
32+
throw new ArgumentNullException("config");
33+
}
34+
35+
_config = config;
36+
_traceWriterLoader = new Lazy<TraceWriter>(() => _config.DependencyResolver.GetService<TraceWriter>());
37+
_handlers = InitializeExceptionHandlers();
38+
}
39+
40+
private static IDictionary<Type, ExceptionProcessor> InitializeExceptionHandlers()
41+
{
42+
var handlers = new Dictionary<Type, ExceptionProcessor>
43+
{
44+
{ typeof(CryptographicException), CryptographicExceptionHandler }
45+
};
46+
47+
return handlers;
48+
}
49+
50+
public override void Handle(ExceptionHandlerContext context)
51+
{
52+
var error = new ApiErrorModel(HttpStatusCode.InternalServerError);
53+
54+
AuthorizationLevel currentLevel = context.Request.GetAuthorizationLevel();
55+
foreach (var handler in GetExceptionHandlers(context.Exception))
56+
{
57+
handler(context.ExceptionContext, currentLevel, error);
58+
}
59+
60+
TraceErrorEvent(context.ExceptionContext, error);
61+
62+
context.Result = new ResponseMessageResult(context.Request.CreateResponse(error.StatusCode, error));
63+
}
64+
65+
private IEnumerable<ExceptionProcessor> GetExceptionHandlers(Exception exc)
66+
{
67+
if (exc == null)
68+
{
69+
yield break;
70+
}
71+
72+
// We return our default handler first.
73+
// Subsequent handlers can override everything done by it.
74+
yield return DefaultExceptionHandler;
75+
76+
Type exceptionType = exc.GetType();
77+
ExceptionProcessor exceptionHandler = null;
78+
if (exceptionType != null && _handlers.TryGetValue(exceptionType, out exceptionHandler))
79+
{
80+
yield return exceptionHandler;
81+
}
82+
}
83+
84+
private static void DefaultExceptionHandler(ExceptionContext exceptionContext, AuthorizationLevel currentLevel, ApiErrorModel error)
85+
{
86+
if (currentLevel == AuthorizationLevel.Admin || exceptionContext.RequestContext.IsLocal)
87+
{
88+
error.Message = GetExceptionMessage(exceptionContext.Exception);
89+
90+
if (exceptionContext.RequestContext.IncludeErrorDetail)
91+
{
92+
error.ErrorDetails = ExceptionFormatter.GetFormattedException(exceptionContext.Exception);
93+
}
94+
}
95+
else
96+
{
97+
error.Message = $"An error has occurred. For more information, please check the logs for error ID {error.Id}";
98+
}
99+
}
100+
101+
private void TraceErrorEvent(ExceptionContext exceptionContext, ApiErrorModel error)
102+
{
103+
string controllerName = exceptionContext.ControllerContext?.ControllerDescriptor.ControllerName ?? "<unknown>";
104+
string actionName = exceptionContext.ActionContext?.ActionDescriptor.ActionName ?? "<unknown>";
105+
106+
string message = JsonConvert.SerializeObject(error);
107+
var traceEvent = new TraceEvent(TraceLevel.Error, message, $"ApiError.{controllerName}.{actionName}", exceptionContext.Exception);
108+
_traceWriterLoader.Value.Trace(traceEvent);
109+
}
110+
111+
private static void CryptographicExceptionHandler(ExceptionContext exceptionContext, AuthorizationLevel currentLevel, ApiErrorModel error)
112+
{
113+
if (currentLevel == AuthorizationLevel.Admin)
114+
{
115+
error.ErrorCode = ErrorCodes.KeyCryptographicError;
116+
error.Message = "Cryptographic error. Unable to encrypt or decrypt keys.";
117+
}
118+
}
119+
120+
private static string GetExceptionMessage(Exception exception)
121+
{
122+
if (exception == null)
123+
{
124+
return string.Empty;
125+
}
126+
127+
var aggregateException = exception as AggregateException;
128+
if (aggregateException != null)
129+
{
130+
exception = aggregateException.Flatten().InnerException;
131+
}
132+
133+
var messages = new List<string>();
134+
while (exception != null)
135+
{
136+
messages.Add(exception.Message);
137+
exception = exception.InnerException;
138+
}
139+
140+
return string.Join(" -> ", messages);
141+
}
142+
}
143+
}

src/WebJobs.Script.WebHost/Handlers/WebScriptHostHandler.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
5151
// initialize. This might happen if http requests come in while the
5252
// host is starting up for the first time, or if it is restarting.
5353
TimeSpan timeWaited = TimeSpan.Zero;
54-
while (!scriptHostManager.CanInvoke() &&
55-
scriptHostManager.State != ScriptHostState.Error &&
54+
while (!scriptHostManager.CanInvoke() &&
55+
scriptHostManager.State != ScriptHostState.Error &&
5656
(timeWaited < _hostTimeoutSeconds))
5757
{
5858
await Task.Delay(_hostRunningPollIntervalMs);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Net;
7+
using Newtonsoft.Json;
8+
9+
namespace Microsoft.Azure.WebJobs.Script.WebHost.Models
10+
{
11+
public class ApiErrorModel
12+
{
13+
public ApiErrorModel(HttpStatusCode status)
14+
: this()
15+
{
16+
StatusCode = status;
17+
}
18+
public ApiErrorModel()
19+
{
20+
Id = Guid.NewGuid().ToString();
21+
}
22+
23+
[JsonProperty("id")]
24+
public string Id { get; set; }
25+
26+
[JsonProperty("statusCode")]
27+
public HttpStatusCode StatusCode { get; set; }
28+
29+
[JsonProperty("errorCode")]
30+
public int ErrorCode { get; set; }
31+
32+
[JsonProperty("messsage", NullValueHandling = NullValueHandling.Ignore)]
33+
public string Message { get; set; }
34+
35+
[JsonProperty("errorDetails", NullValueHandling = NullValueHandling.Ignore)]
36+
public string ErrorDetails { get; set; }
37+
38+
[JsonProperty("arguments", NullValueHandling = NullValueHandling.Ignore)]
39+
public IDictionary<string, string> Arguments { get; set; }
40+
}
41+
}

src/WebJobs.Script.WebHost/WebJobs.Script.WebHost.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@
408408
<Compile Include="Controllers\FunctionsController.cs" />
409409
<Compile Include="Controllers\HomeController.cs" />
410410
<Compile Include="Controllers\KeysController.cs" />
411+
<Compile Include="Handlers\ExceptionProcessingHandler.cs" />
411412
<Compile Include="Controllers\SwaggerController.cs" />
412413
<Compile Include="Diagnostics\EventGenerator.cs" />
413414
<Compile Include="Diagnostics\FastLogger.cs" />
@@ -426,6 +427,7 @@
426427
<Compile Include="GlobalSuppressions.cs" />
427428
<Compile Include="Handlers\SystemTraceHandler.cs" />
428429
<Compile Include="Handlers\WebScriptHostHandler.cs" />
430+
<Compile Include="Models\ApiErrorModel.cs" />
429431
<Compile Include="Models\ApiModel.cs" />
430432
<Compile Include="Models\ApiModelUtility.cs" />
431433
<Compile Include="Models\Swagger\HttpOperationInfo.cs" />

src/WebJobs.Script/ErrorCodes.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
6+
namespace Microsoft.Azure.WebJobs.Script
7+
{
8+
public static class ErrorCodes
9+
{
10+
public const int KeyCryptographicError = 1000;
11+
}
12+
}

src/WebJobs.Script/WebJobs.Script.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@
426426
<Compile Include="Diagnostics\MetricsLogger.cs" />
427427
<Compile Include="Diagnostics\HostStartedEvent.cs" />
428428
<Compile Include="EnvironmentSettingNames.cs" />
429+
<Compile Include="ErrorCodes.cs" />
429430
<Compile Include="Extensions\AssemblyExtensions.cs" />
430431
<Compile Include="Extensions\ExceptionExtensions.cs" />
431432
<Compile Include="Extensions\FileUtility.cs" />
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Net.Http;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using System.Web.Http;
9+
using System.Web.Http.Dependencies;
10+
using System.Web.Http.ExceptionHandling;
11+
using Microsoft.Azure.WebJobs.Host;
12+
using Microsoft.Azure.WebJobs.Script.Config;
13+
using Microsoft.Azure.WebJobs.Script.WebHost.Controllers;
14+
using Microsoft.Azure.WebJobs.Script.WebHost.Models;
15+
using Moq;
16+
using WebJobs.Script.Tests;
17+
using Xunit;
18+
19+
namespace Microsoft.Azure.WebJobs.Script.Tests
20+
{
21+
public class ExceptionProcessingHandlerTests
22+
{
23+
private readonly ScriptSettingsManager _settingsManager;
24+
private readonly TempDirectory _secretsDirectory = new TempDirectory();
25+
private readonly HttpConfiguration _config;
26+
private readonly TestTraceWriter _traceWriter;
27+
28+
public ExceptionProcessingHandlerTests()
29+
{
30+
_settingsManager = ScriptSettingsManager.Instance;
31+
_traceWriter = new TestTraceWriter(System.Diagnostics.TraceLevel.Verbose);
32+
33+
Mock<IDependencyResolver> mockResolver = new Mock<IDependencyResolver>();
34+
mockResolver.Setup(p => p.GetService(typeof(TraceWriter))).Returns(_traceWriter);
35+
36+
_config = new HttpConfiguration();
37+
_config.DependencyResolver = mockResolver.Object;
38+
}
39+
40+
[Fact]
41+
public async Task Handle_WithAnonymousLevelAndNoErrorDetailsFlag_SetsResponseWithNoErrorDetailsAndGenericMessage()
42+
{
43+
var errorModel = await ExecuteHandlerTest(AuthorizationLevel.Anonymous, false);
44+
45+
Assert.Null(errorModel.ErrorDetails);
46+
Assert.Equal($"An error has occurred. For more information, please check the logs for error ID {errorModel.Id}", errorModel.Message);
47+
}
48+
49+
[Fact]
50+
public async Task Handle_WithAdminlAndNoErrorDetailsFlag_SetsResponseWithNoErrorDetailsAndExceptionMessage()
51+
{
52+
var errorModel = await ExecuteHandlerTest(AuthorizationLevel.Admin, false);
53+
54+
Assert.Null(errorModel.ErrorDetails);
55+
Assert.Equal("TestException", errorModel.Message);
56+
}
57+
58+
[Fact]
59+
public async Task Handle_WithAdminlAndErrorDetailsFlag_SetsResponseWithErrorDetailsAndExceptionMessage()
60+
{
61+
var errorModel = await ExecuteHandlerTest(AuthorizationLevel.Admin, true);
62+
63+
Assert.Equal("System.Exception : TestException", errorModel.ErrorDetails);
64+
Assert.Equal("TestException", errorModel.Message);
65+
}
66+
67+
[Fact]
68+
public async Task Handle_WithAnonymousAndErrorDetailsFlag_SetsResponseWithNoErrorDetailsAndExceptionMessage()
69+
{
70+
var errorModel = await ExecuteHandlerTest(AuthorizationLevel.Anonymous, true);
71+
72+
Assert.Null(errorModel.ErrorDetails);
73+
Assert.Equal($"An error has occurred. For more information, please check the logs for error ID {errorModel.Id}", errorModel.Message);
74+
}
75+
76+
private async Task<ApiErrorModel> ExecuteHandlerTest(AuthorizationLevel authLevel, bool includeDetails)
77+
{
78+
var handler = new ExceptionProcessingHandler(_config);
79+
80+
var exception = new Exception("TestException");
81+
var exceptionContext = new ExceptionContext(exception, ExceptionCatchBlocks.HttpServer)
82+
{
83+
Request = new HttpRequestMessage()
84+
};
85+
86+
exceptionContext.Request.SetAuthorizationLevel(authLevel);
87+
exceptionContext.Request.SetConfiguration(_config);
88+
89+
exceptionContext.RequestContext = new System.Web.Http.Controllers.HttpRequestContext();
90+
91+
var context = new ExceptionHandlerContext(exceptionContext);
92+
context.RequestContext.IncludeErrorDetail = includeDetails;
93+
94+
handler.Handle(context);
95+
96+
HttpResponseMessage response = await context.Result.ExecuteAsync(CancellationToken.None);
97+
98+
return await response.Content.ReadAsAsync<ApiErrorModel>();
99+
}
100+
}
101+
}

test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,7 @@
470470
<Compile Include="Description\FunctionGeneratorTests.cs" />
471471
<Compile Include="Description\DotNet\PackageAssemblyResolverTests.cs" />
472472
<Compile Include="Description\FunctionInvokerBaseTests.cs" />
473+
<Compile Include="Handlers\ExceptionProcessingHandlerTests.cs" />
473474
<Compile Include="Handlers\SystemTraceHandlerTests.cs" />
474475
<Compile Include="HttpRouteFactoryTests.cs" />
475476
<Compile Include="Binding\HttpTriggerAttributeBindingProviderTests.cs" />

0 commit comments

Comments
 (0)