From 1f881b7c1f19078847b857423bad2d19f50e4143 Mon Sep 17 00:00:00 2001 From: Drew Gillies Date: Thu, 13 Jun 2024 14:18:45 +1000 Subject: [PATCH 1/4] Remove elmah table usage from gallery --- CREDITS.txt | 12 - sign.thirdparty.props | 1 - .../Configuration/GalleryConfiguration.cs | 1 - .../Infrastructure/ElmahException.cs | 49 ---- .../Infrastructure/TableErrorLog.cs | 236 ------------------ .../NuGetGallery.Core.csproj | 3 - .../Configuration/AppConfiguration.cs | 3 - .../Configuration/IAppConfiguration.cs | 5 - .../Diagnostics/ElmahHandleErrorAttribute.cs | 64 ----- .../SendErrorsToTelemetryAttribute.cs | 7 +- .../Telemetry/QuietLog.cs | 57 +---- src/NuGetGallery/App_Start/AppActivator.cs | 4 +- .../App_Start/DefaultDependenciesModule.cs | 15 -- src/NuGetGallery/App_Start/OwinStartup.cs | 23 -- .../Areas/Admin/AdminAreaRegistration.cs | 2 - .../Mail/BackgroundMarkdownMessageService.cs | 9 +- .../Services/JsonStatisticsService.cs | 5 +- src/NuGetGallery/Web.config | 61 +---- .../Infrastructure/ElmahExceptionFacts.cs | 43 ---- .../Infrastructure/TableErrorLogFacts.cs | 48 ---- .../NuGetGallery.Core.Facts.csproj | 2 - .../Constants.cs | 1 - 22 files changed, 10 insertions(+), 641 deletions(-) delete mode 100644 src/NuGetGallery.Core/Infrastructure/ElmahException.cs delete mode 100644 src/NuGetGallery.Core/Infrastructure/TableErrorLog.cs delete mode 100644 src/NuGetGallery.Services/Diagnostics/ElmahHandleErrorAttribute.cs delete mode 100644 tests/NuGetGallery.Core.Facts/Infrastructure/ElmahExceptionFacts.cs delete mode 100644 tests/NuGetGallery.Core.Facts/Infrastructure/TableErrorLogFacts.cs diff --git a/CREDITS.txt b/CREDITS.txt index 892c5df20b..d011f45d72 100644 --- a/CREDITS.txt +++ b/CREDITS.txt @@ -6,18 +6,6 @@ The .NET Foundation (http://www.dotnetfoundation.org). NuGet includes works distributed under the licenses listed below. The full text for most of the licenses listed below can be found in the LICENSE.txt file accompanying each work. The original copyright notices have been preserved within the respective files and or packages. Please refer to the specific files and/or packages for more detailed information about the authors, copyright notices, and licenses. -Elmah 1.1 ------ -Website: http://code.google.com/p/elmah/ -Copyright: Copyright (c) 2010 Atif Azis -License: Apache 2.0 - -Elmah.Contrib.Mvc 1.0 ----- -Website: http://nuget.org/List/Packages/Elmah.Contrib.Mvc -Copyright: Copyright (c) 2011, Fabian Vilers -License: Ms-PL - jQuery 1.6.2 ---- Website: http://jquery.com/ diff --git a/sign.thirdparty.props b/sign.thirdparty.props index 2989ea5af8..c89ab14edf 100644 --- a/sign.thirdparty.props +++ b/sign.thirdparty.props @@ -13,7 +13,6 @@ - diff --git a/src/AccountDeleter/Configuration/GalleryConfiguration.cs b/src/AccountDeleter/Configuration/GalleryConfiguration.cs index 254963649c..504a90a6c2 100644 --- a/src/AccountDeleter/Configuration/GalleryConfiguration.cs +++ b/src/AccountDeleter/Configuration/GalleryConfiguration.cs @@ -28,7 +28,6 @@ public string SiteRoot public string AzureStorage_Auditing_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } public string AzureStorage_UserCertificates_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } public string AzureStorage_Content_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public string AzureStorage_Errors_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } public string AzureStorage_Packages_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } public string AzureStorage_FlatContainer_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } public string AzureStorage_Statistics_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } diff --git a/src/NuGetGallery.Core/Infrastructure/ElmahException.cs b/src/NuGetGallery.Core/Infrastructure/ElmahException.cs deleted file mode 100644 index 41470762f8..0000000000 --- a/src/NuGetGallery.Core/Infrastructure/ElmahException.cs +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections; -using System.Collections.Generic; - -namespace NuGetGallery -{ - /// - /// An exception to be used for Elmah logs. - /// - public class ElmahException : Exception - { - private readonly Exception _baseException; - - /// - /// Server variables values in Elmah logs will be overwritten by these values. - /// - public Dictionary ServerVariables - { - get; - } - - public ElmahException(Exception e, Dictionary serverVariables) : base(e.Message, e.InnerException) - { - ServerVariables = serverVariables ?? new Dictionary(); - _baseException = e; - } - - public override string StackTrace => _baseException.StackTrace; - - public override IDictionary Data => _baseException.Data; - - public override string Source { get => _baseException.Source; set => Source = value; } - - public override Exception GetBaseException() - { - return _baseException.GetBaseException(); - } - - public override string HelpLink { get => _baseException.HelpLink; set => HelpLink = value; } - - public override string ToString() - { - return _baseException.ToString(); - } - } -} \ No newline at end of file diff --git a/src/NuGetGallery.Core/Infrastructure/TableErrorLog.cs b/src/NuGetGallery.Core/Infrastructure/TableErrorLog.cs deleted file mode 100644 index 8cefe9c7f1..0000000000 --- a/src/NuGetGallery.Core/Infrastructure/TableErrorLog.cs +++ /dev/null @@ -1,236 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections; -using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; -using System.Linq; -using Elmah; -using Microsoft.WindowsAzure.Storage; -using Microsoft.WindowsAzure.Storage.Table; -using NuGetGallery.Auditing; - -namespace NuGetGallery.Infrastructure -{ - public class ErrorEntity : ITableEntity - { - public Error Error { get; set; } - - string ITableEntity.ETag - { - get; - set; - } - - string ITableEntity.PartitionKey - { - get; - set; - } - - string ITableEntity.RowKey - { - get; - set; - } - - DateTimeOffset ITableEntity.Timestamp - { - get; - set; - } - - public long LogicalIndex - { - get - { - return AzureEntityList.GetLogicalIndex(this); - } - } - - public ErrorEntity() { } - - public ErrorEntity(Error error) - { - Error = error; - } - - void ITableEntity.ReadEntity(IDictionary properties, Microsoft.WindowsAzure.Storage.OperationContext operationContext) - { - // This can occasionally fail because someone didn't finish creating the entity yet. - - EntityProperty value; - if (properties.TryGetValue("SerializedError", out value)) - { - Error = ErrorXml.DecodeString(value.StringValue); - } - else - { - Error = new Error - { - ApplicationName = "TableErrorLog", - StatusCode = 999, - HostName = Environment.MachineName, - Time = DateTime.UtcNow, - Type = typeof(Exception).FullName, - Detail = "Error Log Entry is Corrupted/Missing in Table Store" - }; - - return; - } - - if (properties.TryGetValue("Detail", out value)) - { - Error.Detail = value.StringValue; - } - - if (properties.TryGetValue("WebHostHtmlMessage", out value)) - { - Error.WebHostHtmlMessage = value.StringValue; - } - } - - IDictionary ITableEntity.WriteEntity(OperationContext operationContext) - { - // Table storage has a limitation on property lengths - 64KiB. - // Strings will be encoded as UTF-16, apparently? - - const int MaxChars = 32 * 1000; - - var detail = Error.Detail; - if (detail.Length > MaxChars) - { - detail = detail.Substring(0, MaxChars); - } - - var htmlMessage = Error.WebHostHtmlMessage; - if (htmlMessage.Length > MaxChars) - { - htmlMessage = htmlMessage.Substring(0, MaxChars); - } - - Error.Detail = null; - Error.WebHostHtmlMessage = null; - string serializedError = ErrorXml.EncodeString(Error); - - if (serializedError.Length > MaxChars) - { - serializedError = ErrorXml.EncodeString( - new Error - { - ApplicationName = "TableErrorLog", - StatusCode = 888, - HostName = Environment.MachineName, - Time = DateTime.UtcNow, - Detail = "Error Log Entry Will Not Fit In Table Store: " + serializedError.Substring(0, 4000) - }); - } - - return new Dictionary - { - { "SerializedError", EntityProperty.GeneratePropertyForString(serializedError) }, - { "Detail", EntityProperty.GeneratePropertyForString(detail) }, - { "WebHostHtmlMessage", EntityProperty.GeneratePropertyForString(htmlMessage) }, - }; - } - } - - public class TableErrorLog : ErrorLog - { - public const string TableName = "ElmahErrors"; - - private readonly AzureEntityList _entityList; - - public TableErrorLog(Func connectionStringFactory, bool readAccessGeoRedundant) - { - _entityList = new AzureEntityList(connectionStringFactory, TableName, readAccessGeoRedundant); - } - - public override ErrorLogEntry GetError(string id) - { - long pos = Int64.Parse(id, CultureInfo.InvariantCulture); - var error = _entityList[pos]; - Debug.Assert(id == pos.ToString(CultureInfo.InvariantCulture)); - return new ErrorLogEntry(this, id, error.Error); - } - - public override int GetErrors(int pageIndex, int pageSize, IList errorEntryList) - { - // A little math is required since the AzureEntityList is in ascending order - // And we want to retrieve entries in descending order - long queryOffset = _entityList.LongCount - ((pageIndex + 1) * pageSize); - if (queryOffset < 0) - { - pageSize += (int)queryOffset; - queryOffset = 0; - } - - // And since that range was in ascending, flip it to descending. - var results = _entityList.GetRange(queryOffset, pageSize).Reverse(); - foreach (var error in results) - { - string id = error.LogicalIndex.ToString(CultureInfo.InvariantCulture); - errorEntryList.Add(new ErrorLogEntry(this, id, error.Error)); - } - - return _entityList.Count; - } - - public override string Log(Error error) - { - Obfuscate(error); - var entity = new ErrorEntity(error); - long pos = _entityList.Add(entity); - return pos.ToString(CultureInfo.InvariantCulture); - } - - public static void Obfuscate(Error error) - { - error.User = string.Empty; - if (error.Form != null) - { - error.Form.Clear(); - } - - //ServerVariables overrides requiring context from the http request should be handled in NuGetGallery.QuietLog - var elmahException = error.Exception as ElmahException; - if (elmahException != null) - { - var piiServerVaribles = elmahException.ServerVariables; - foreach (var key in piiServerVaribles.Keys) - { - error.ServerVariables[key] = piiServerVaribles[key]; - } - } - - error.ServerVariables["ALL_HTTP"] = string.Empty; - error.ServerVariables["ALL_RAW"] = string.Empty; - - error.ServerVariables["AUTH_USER"] = string.Empty; - error.ServerVariables["LOGON_USER"] = string.Empty; - error.ServerVariables["REMOTE_USER"] = string.Empty; - - error.ServerVariables["REMOTE_ADDR"] = Obfuscator.ObfuscateIp(error.ServerVariables["REMOTE_ADDR"]); - error.ServerVariables["REMOTE_HOST"] = Obfuscator.ObfuscateIp(error.ServerVariables["REMOTE_HOST"]); - error.ServerVariables["LOCAL_ADDR"] = Obfuscator.ObfuscateIp(error.ServerVariables["LOCAL_ADDR"]); - - error.ServerVariables["HTTP_X_NUGET_APIKEY"] = string.Empty; - - var forwardedIps = error.ServerVariables["HTTP_X_FORWARDED_FOR"]? - .Split(',') - .Select(x => x.Trim()) - .Where(x => x.Length > 0) - .ToList(); - if (forwardedIps != null) - { - var obfuscatedIps = string.Join(",", forwardedIps.Select(Obfuscator.ObfuscateIp)); - if (!string.IsNullOrWhiteSpace(obfuscatedIps)) - { - error.ServerVariables["HTTP_X_FORWARDED_FOR"] = obfuscatedIps; - } - } - } - } -} \ No newline at end of file diff --git a/src/NuGetGallery.Core/NuGetGallery.Core.csproj b/src/NuGetGallery.Core/NuGetGallery.Core.csproj index 4b8b74c4b4..221a3609d7 100644 --- a/src/NuGetGallery.Core/NuGetGallery.Core.csproj +++ b/src/NuGetGallery.Core/NuGetGallery.Core.csproj @@ -65,9 +65,6 @@ $(ServerCommonPackageVersion) - - 1.2.2 - 5.8.4 diff --git a/src/NuGetGallery.Services/Configuration/AppConfiguration.cs b/src/NuGetGallery.Services/Configuration/AppConfiguration.cs index 152432de29..f3eed8fb4f 100644 --- a/src/NuGetGallery.Services/Configuration/AppConfiguration.cs +++ b/src/NuGetGallery.Services/Configuration/AppConfiguration.cs @@ -48,9 +48,6 @@ public class AppConfiguration : IAppConfiguration [DisplayName("AzureStorage.Content.ConnectionString")] public string AzureStorage_Content_ConnectionString { get; set; } - [DisplayName("AzureStorage.Errors.ConnectionString")] - public string AzureStorage_Errors_ConnectionString { get; set; } - [DisplayName("AzureStorage.Packages.ConnectionString")] public string AzureStorage_Packages_ConnectionString { get; set; } diff --git a/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs b/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs index bc15a407f4..452e39913c 100644 --- a/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs +++ b/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs @@ -55,11 +55,6 @@ public interface IAppConfiguration : IMessageServiceConfiguration /// string AzureStorage_Content_ConnectionString { get; set; } - /// - /// The Azure Storage connection string used for Elmah error logs. - /// - string AzureStorage_Errors_ConnectionString { get; set; } - /// /// The Azure Storage connection string used for packages, after upload. /// diff --git a/src/NuGetGallery.Services/Diagnostics/ElmahHandleErrorAttribute.cs b/src/NuGetGallery.Services/Diagnostics/ElmahHandleErrorAttribute.cs deleted file mode 100644 index ee50f25e41..0000000000 --- a/src/NuGetGallery.Services/Diagnostics/ElmahHandleErrorAttribute.cs +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Web; -using System.Web.Mvc; -using Elmah; - -namespace NuGetGallery.Diagnostics -{ - /// - /// Source: http://stackoverflow.com/a/779961/52749 - /// - public class ElmahHandleErrorAttribute : HandleErrorAttribute - { - public override void OnException(ExceptionContext context) - { - base.OnException(context); - if (!context.ExceptionHandled // if unhandled, will be logged anyhow - || TryRaiseErrorSignal(context) // prefer signaling, if possible - || IsFiltered(context)) // filtered? - return; - - LogException(context); - } - - private static bool TryRaiseErrorSignal(ExceptionContext context) - { - var httpContext = GetHttpContextImpl(context.HttpContext); - if (httpContext == null) - return false; - var signal = ErrorSignal.FromContext(httpContext); - if (signal == null) - return false; - signal.Raise(context.Exception, httpContext); - return true; - } - - private static bool IsFiltered(ExceptionContext context) - { - var config = context.HttpContext.GetSection("elmah/errorFilter") - as ErrorFilterConfiguration; - - if (config == null) - return false; - - var testContext = new ErrorFilterModule.AssertionHelperContext( - context.Exception, - GetHttpContextImpl(context.HttpContext)); - return config.Assertion.Test(testContext); - } - - private static void LogException(ExceptionContext context) - { - var httpContext = GetHttpContextImpl(context.HttpContext); - var error = new Error(context.Exception, httpContext); - ErrorLog.GetDefault(httpContext).Log(error); - } - - private static HttpContext GetHttpContextImpl(HttpContextBase context) - { - return context.ApplicationInstance.Context; - } - } -} \ No newline at end of file diff --git a/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs b/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs index f39af59451..5003847ea9 100644 --- a/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs +++ b/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs @@ -8,13 +8,10 @@ namespace NuGetGallery.Diagnostics { [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] - public sealed class SendErrorsToTelemetryAttribute - : ElmahHandleErrorAttribute + public sealed class SendErrorsToTelemetryAttribute : Attribute { - public override void OnException(ExceptionContext context) + public void OnException(ExceptionContext context) { - base.OnException(context); - if (context != null) { try diff --git a/src/NuGetGallery.Services/Telemetry/QuietLog.cs b/src/NuGetGallery.Services/Telemetry/QuietLog.cs index 0879b48404..d567422703 100644 --- a/src/NuGetGallery.Services/Telemetry/QuietLog.cs +++ b/src/NuGetGallery.Services/Telemetry/QuietLog.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Web; using System.Web.Routing; -using Elmah; namespace NuGetGallery { @@ -43,64 +42,10 @@ public static void LogHandledException(Exception e) } } - public static void LogHandledException(Exception e, ErrorLog errorLog) - { - var aggregateExceptionId = Guid.NewGuid().ToString(); - - var aggregateException = e as AggregateException; - if (aggregateException != null) - { - LogHandledExceptionCore(aggregateException, aggregateExceptionId, errorLog); - - foreach (var innerException in aggregateException.InnerExceptions) - { - LogHandledExceptionCore(innerException, aggregateExceptionId, errorLog); - } - } - else - { - LogHandledExceptionCore(e, aggregateExceptionId, errorLog); - - if (e.InnerException != null) - { - LogHandledExceptionCore(e.InnerException, aggregateExceptionId, errorLog); - } - } - } - private static void LogHandledExceptionCore(Exception e, string aggregateExceptionId) { try { - var currentHttpContext = HttpContext.Current; - if (currentHttpContext != null) - { - var elmahException = new ElmahException(e, GetObfuscatedServerVariables(new HttpContextWrapper(currentHttpContext))); - ErrorSignal.FromCurrentContext().Raise(elmahException); - } - else - { - ErrorLog.GetDefault(null).Log(new Error(e)); - } - - // send exception to AppInsights - Telemetry.TrackException(e, new Dictionary - { - { "aggregateExceptionId", aggregateExceptionId } - }); - } - catch - { - // logging failed, don't allow exception to escape - } - } - - private static void LogHandledExceptionCore(Exception e, string aggregateExceptionId, ErrorLog errorLog) - { - try - { - errorLog.Log(new Error(e)); - // send exception to AppInsights Telemetry.TrackException(e, new Dictionary { @@ -125,7 +70,7 @@ internal static bool IsPIIRoute(RouteData route, out string operation) } /// - /// These values will be used to overwrite the serverVariables in the Elmah error before the exception is logged. + /// These values will be used to overwrite the serverVariables in the error before the exception is logged. /// These are the serverVariables that Gallery decides that will be obfuscated and the values to be used for obfuscation. /// /// The current HttpContext. diff --git a/src/NuGetGallery/App_Start/AppActivator.cs b/src/NuGetGallery/App_Start/AppActivator.cs index e52f17b4d3..6903ac30e5 100644 --- a/src/NuGetGallery/App_Start/AppActivator.cs +++ b/src/NuGetGallery/App_Start/AppActivator.cs @@ -13,7 +13,6 @@ using System.Web.Optimization; using System.Web.Routing; using System.Web.UI; -using Elmah; using Microsoft.Extensions.DependencyInjection; using NuGetGallery; using NuGetGallery.Configuration; @@ -253,7 +252,6 @@ private static void AppPostStart(IAppConfiguration configuration) Routes.RegisterRoutes(RouteTable.Routes, configuration.FeedOnlyMode, configuration.AdminPanelEnabled); AreaRegistration.RegisterAllAreas(); - GlobalFilters.Filters.Add(new SendErrorsToTelemetryAttribute { View = "~/Views/Errors/InternalError.cshtml" }); GlobalFilters.Filters.Add(new ReadOnlyModeErrorFilter()); GlobalFilters.Filters.Add(new AntiForgeryErrorFilter()); GlobalFilters.Filters.Add(new UserDeletedErrorFilter()); @@ -295,7 +293,7 @@ private static void BackgroundJobsPostStart(IAppConfiguration configuration) { RestartSchedulerOnFailure = true }; - _jobManager.Fail(e => ErrorLog.GetDefault(null).Log(new Error(e))); + _jobManager.Fail(e => { /* fail silently */ }); _jobManager.Start(); } } diff --git a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs index 9c6e536a35..55695306de 100644 --- a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs +++ b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs @@ -20,7 +20,6 @@ using Autofac; using Autofac.Core; using Autofac.Extensions.DependencyInjection; -using Elmah; using Microsoft.ApplicationInsights.Extensibility; using Microsoft.ApplicationInsights.Extensibility.Implementation; using Microsoft.Extensions.DependencyInjection; @@ -612,8 +611,6 @@ internal static ApplicationInsightsConfiguration ConfigureApplicationInsights( telemetryConfiguration.TelemetryProcessorChainBuilder.Build(); - QuietLog.UseTelemetryClient(telemetryClientWrapper); - telemetryClient = telemetryClientWrapper; return applicationInsightsConfiguration; @@ -1408,10 +1405,6 @@ private static void ConfigureForLocalFileSystem(ContainerBuilder builder, IGalle .As() .InstancePerLifetimeScope(); - builder.RegisterInstance(new SqlErrorLog(configuration.Current.SqlConnectionString)) - .As() - .SingleInstance(); - builder.RegisterType() .As() .SingleInstance(); @@ -1477,14 +1470,6 @@ private static void ConfigureForAzureStorage(ContainerBuilder builder, IGalleryC RegisterStatisticsServices(builder, configuration); - builder.Register(c => - { - var configurationFactory = c.Resolve>(); - return new TableErrorLog(() => configurationFactory().AzureStorage_Errors_ConnectionString, configurationFactory().AzureStorageReadAccessGeoRedundant); - }) - .As() - .SingleInstance(); - builder.RegisterType() .As() .SingleInstance(); diff --git a/src/NuGetGallery/App_Start/OwinStartup.cs b/src/NuGetGallery/App_Start/OwinStartup.cs index 17380bc3e0..861e637874 100644 --- a/src/NuGetGallery/App_Start/OwinStartup.cs +++ b/src/NuGetGallery/App_Start/OwinStartup.cs @@ -13,7 +13,6 @@ using System.Web.Hosting; using System.Web.Http; using System.Web.Mvc; -using Elmah; using Microsoft.Owin; using Microsoft.Owin.Logging; using Microsoft.Owin.Security; @@ -55,10 +54,6 @@ public static void Configuration(IAppBuilder app) app.UseAutofacInjection(GlobalConfiguration.Configuration); var dependencyResolver = DependencyResolver.Current; - // Register Elmah - var elmahServiceCenter = new DependencyResolverServiceProviderAdapter(dependencyResolver); - ServiceCenter.Current = _ => elmahServiceCenter; - // Get config var config = dependencyResolver.GetService(); var auth = dependencyResolver.GetService(); @@ -180,24 +175,6 @@ public static void Configuration(IAppBuilder app) // this is a tragic moment... swallow Exception to prevent crashing IIS } - // Send to ELMAH - try - { - HttpContext current = HttpContext.Current; - if (current != null) - { - var errorSignal = ErrorSignal.FromContext(current); - if (errorSignal != null) - { - errorSignal.Raise(exArgs.Exception, current); - } - } - } - catch (Exception) - { - // more tragedy... swallow Exception to prevent crashing IIS - } - exArgs.SetObserved(); }; diff --git a/src/NuGetGallery/Areas/Admin/AdminAreaRegistration.cs b/src/NuGetGallery/Areas/Admin/AdminAreaRegistration.cs index f0cbbd099a..738b8e8253 100644 --- a/src/NuGetGallery/Areas/Admin/AdminAreaRegistration.cs +++ b/src/NuGetGallery/Areas/Admin/AdminAreaRegistration.cs @@ -29,8 +29,6 @@ public override void RegisterArea(AreaRegistrationContext context) DynamicDataManager.Register(context.Routes, "Admin/Database", galleryDbConnectionFactory); } - context.Routes.Ignore("Admin/Errors.axd/{*pathInfo}"); // ELMAH owns this root - context.MapRoute( "Admin_default", "Admin/{controller}/{action}/{id}", diff --git a/src/NuGetGallery/Infrastructure/Mail/BackgroundMarkdownMessageService.cs b/src/NuGetGallery/Infrastructure/Mail/BackgroundMarkdownMessageService.cs index 49c3daca15..240b7f9a20 100644 --- a/src/NuGetGallery/Infrastructure/Mail/BackgroundMarkdownMessageService.cs +++ b/src/NuGetGallery/Infrastructure/Mail/BackgroundMarkdownMessageService.cs @@ -6,7 +6,6 @@ using System.Threading.Tasks; using System.Web.Hosting; using AnglicanGeek.MarkdownMailer; -using Elmah; using NuGetGallery.Configuration; namespace NuGetGallery.Infrastructure.Mail @@ -17,16 +16,13 @@ public BackgroundMarkdownMessageService( IMailSender mailSender, IAppConfiguration config, ITelemetryService telemetryService, - ErrorLog errorLog, Func messageServiceFactory) : base(mailSender, config, telemetryService) { - _errorLog = errorLog ?? throw new ArgumentNullException(nameof(errorLog)); _messageServiceFactory = messageServiceFactory ?? throw new ArgumentNullException(nameof(messageServiceFactory)); _sentMessage = false; } - private readonly ErrorLog _errorLog; private Func _messageServiceFactory; private bool _sentMessage; @@ -57,10 +53,9 @@ protected override Task SendMessageInternalAsync(MailMessage mailMessage) { await base.SendMessageInternalAsync(messageCopy); } - catch (Exception ex) + catch (Exception) { - // Log but swallow the exception. - QuietLog.LogHandledException(ex, _errorLog); + // Swallow the exception. } finally { diff --git a/src/NuGetGallery/Services/JsonStatisticsService.cs b/src/NuGetGallery/Services/JsonStatisticsService.cs index d73699c849..acd2efadd7 100644 --- a/src/NuGetGallery/Services/JsonStatisticsService.cs +++ b/src/NuGetGallery/Services/JsonStatisticsService.cs @@ -205,9 +205,8 @@ private async Task LoadDownloadPackages( return StatisticsReportResult.Success(reportContent.LastUpdatedUtc); } - catch (Exception e) + catch (Exception) { - QuietLog.LogHandledException(e); return StatisticsReportResult.Failed; } } @@ -372,7 +371,7 @@ public async Task GetPackageDownloadsByVersion(string catch (StatisticsReportNotFoundException) { //do no logging and just return null. Since this exception will thrown for all packages which doesn't have downloads in last 6 weeks, we don't - //want to flood the elmah logs. + //want to flood the logs. return null; } catch (NullReferenceException e) diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config index 9f28f8d531..c15c6a9f0a 100644 --- a/src/NuGetGallery/Web.config +++ b/src/NuGetGallery/Web.config @@ -9,12 +9,6 @@
- -
-
-
-
-
@@ -215,51 +209,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -325,7 +274,7 @@ --> - + @@ -346,9 +295,6 @@ - - - @@ -396,9 +342,6 @@ - - - @@ -720,4 +663,4 @@ - + \ No newline at end of file diff --git a/tests/NuGetGallery.Core.Facts/Infrastructure/ElmahExceptionFacts.cs b/tests/NuGetGallery.Core.Facts/Infrastructure/ElmahExceptionFacts.cs deleted file mode 100644 index d8371345dc..0000000000 --- a/tests/NuGetGallery.Core.Facts/Infrastructure/ElmahExceptionFacts.cs +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using Xunit; - -namespace NuGetGallery.Infrastructure -{ - public class ElmahExceptionFacts - { - [Fact] - public void ValidElmahException() - { - // Arrange & Act - var exception = new Exception("Boo", new Exception("Inner Boo")); - Dictionary serverVariables = new Dictionary(); - serverVariables.Add("AUTH_USER", "booUser"); - var elmahException = new ElmahException(exception, serverVariables); - - // Assert - Assert.Equal("booUser", elmahException.ServerVariables["AUTH_USER"]); - Assert.Equal("Inner Boo", elmahException.InnerException.Message); - Assert.Equal("Boo", elmahException.Message); - Assert.Equal(elmahException.Message, exception.Message); - Assert.Equal(elmahException.ToString(), exception.ToString()); - Assert.Equal(elmahException.InnerException.ToString(), exception.InnerException.ToString()); - Assert.Equal(elmahException.GetBaseException(), exception.GetBaseException()); - Assert.Equal(elmahException.HelpLink, exception.HelpLink); - } - - [Fact] - public void NullElmahExceptionServerVariables() - { - // Arrange & Act - var exception = new Exception("Boo", new Exception("Inner Boo")); - var elmahException = new ElmahException(exception, null); - - // Assert - Assert.Empty(elmahException.ServerVariables.Keys); - } - } -} diff --git a/tests/NuGetGallery.Core.Facts/Infrastructure/TableErrorLogFacts.cs b/tests/NuGetGallery.Core.Facts/Infrastructure/TableErrorLogFacts.cs deleted file mode 100644 index 31bf7d1aae..0000000000 --- a/tests/NuGetGallery.Core.Facts/Infrastructure/TableErrorLogFacts.cs +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Linq; -using Elmah; -using Xunit; - -namespace NuGetGallery.Infrastructure -{ - public class TableErrorLogFacts - { - public class TheObfuscateMethod - { - [Fact] - public void HandlesMissingForwardedHeader() - { - // Arrange - var error = new Error(); - - // Act - TableErrorLog.Obfuscate(error); - - // Assert - Assert.DoesNotContain("HTTP_X_FORWARDED_FOR", error.ServerVariables.Keys.Cast()); - } - - [Theory] - [InlineData("", "")] - [InlineData(",", ",")] - [InlineData(" ", " ")] - [InlineData("127.0.0.1", "127.0.0.0")] - [InlineData("127.1.2.3,127.1.2.4", "127.1.2.0,127.1.2.0")] - [InlineData("127.1.2.3 , 127.1.2.4", "127.1.2.0,127.1.2.0")] - public void ObfuscatesForwardedHeader(string input, string expected) - { - // Arrange - var error = new Error(); - error.ServerVariables["HTTP_X_FORWARDED_FOR"] = input; - - // Act - TableErrorLog.Obfuscate(error); - - // Assert - Assert.Equal(expected, error.ServerVariables["HTTP_X_FORWARDED_FOR"]); - } - } - } -} diff --git a/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj b/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj index 04b06ca50a..02a71cc4ca 100644 --- a/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj +++ b/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj @@ -87,7 +87,6 @@ - @@ -99,7 +98,6 @@ - diff --git a/tests/NuGetGallery.FunctionalTests.Core/Constants.cs b/tests/NuGetGallery.FunctionalTests.Core/Constants.cs index e4761fc2a9..d2bed460e9 100644 --- a/tests/NuGetGallery.FunctionalTests.Core/Constants.cs +++ b/tests/NuGetGallery.FunctionalTests.Core/Constants.cs @@ -27,7 +27,6 @@ public static class Constants public const string RegisterNewUserConfirmationText = "Your account is now registered!"; public const string UserAlreadyExistsText = "User already exists"; public const string ReadOnlyModeRegisterNewUserText = "503 : Please try again later! (Read-only)"; - public const string SearchTerm = "elmah"; public const string CreateNewAccountText = "Create A New Account"; public const string StatsPageDefaultText = "Statistics last updated"; public const string ContactOwnersText = "Your message has been sent to the owners of"; From 4c4252d08c7dc76ca3a5fed834512910a92b0635 Mon Sep 17 00:00:00 2001 From: Drew Gillies Date: Fri, 14 Jun 2024 08:28:21 +1000 Subject: [PATCH 2/4] Address feedback --- src/NuGetGallery/Web.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config index c15c6a9f0a..76e674822d 100644 --- a/src/NuGetGallery/Web.config +++ b/src/NuGetGallery/Web.config @@ -274,7 +274,7 @@ --> - + From cf1593f269ccd74dc3611b4fedd20bc5c4bb4c6d Mon Sep 17 00:00:00 2001 From: Drew Gillies Date: Fri, 21 Jun 2024 15:53:02 +1000 Subject: [PATCH 3/4] Addressed feedback, reintroduce elmah dll signing to support migrations --- sign.thirdparty.props | 1 + src/NuGetGallery/App_Start/AppActivator.cs | 2 +- src/NuGetGallery/Areas/Admin/Views/Home/Index.cshtml | 11 ----------- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/sign.thirdparty.props b/sign.thirdparty.props index c89ab14edf..2989ea5af8 100644 --- a/sign.thirdparty.props +++ b/sign.thirdparty.props @@ -13,6 +13,7 @@ + diff --git a/src/NuGetGallery/App_Start/AppActivator.cs b/src/NuGetGallery/App_Start/AppActivator.cs index 6903ac30e5..1e6d8cfa2d 100644 --- a/src/NuGetGallery/App_Start/AppActivator.cs +++ b/src/NuGetGallery/App_Start/AppActivator.cs @@ -293,7 +293,7 @@ private static void BackgroundJobsPostStart(IAppConfiguration configuration) { RestartSchedulerOnFailure = true }; - _jobManager.Fail(e => { /* fail silently */ }); + _jobManager.Fail(e => { Trace.TraceError($"{nameof(BackgroundJobsPostStart)} failure: {e.Message}"); }); _jobManager.Start(); } } diff --git a/src/NuGetGallery/Areas/Admin/Views/Home/Index.cshtml b/src/NuGetGallery/Areas/Admin/Views/Home/Index.cshtml index 200abcd523..561e671230 100644 --- a/src/NuGetGallery/Areas/Admin/Views/Home/Index.cshtml +++ b/src/NuGetGallery/Areas/Admin/Views/Home/Index.cshtml @@ -22,17 +22,6 @@

} -
  • -

    - - - Error logs - -

    -

    - View application error logs. -

    -

  • From f743704056c9f0586288cba9531bc74fcc0134dc Mon Sep 17 00:00:00 2001 From: Drew Gillies Date: Mon, 24 Jun 2024 15:49:16 +1000 Subject: [PATCH 4/4] Correct telemetry base class - this change was breaking functional tests --- .../Diagnostics/SendErrorsToTelemetryAttribute.cs | 7 +++++-- src/NuGetGallery/App_Start/AppActivator.cs | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs b/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs index 5003847ea9..7300418fac 100644 --- a/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs +++ b/src/NuGetGallery.Services/Diagnostics/SendErrorsToTelemetryAttribute.cs @@ -8,10 +8,13 @@ namespace NuGetGallery.Diagnostics { [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] - public sealed class SendErrorsToTelemetryAttribute : Attribute + public sealed class SendErrorsToTelemetryAttribute : + HandleErrorAttribute { - public void OnException(ExceptionContext context) + public override void OnException(ExceptionContext context) { + base.OnException(context); + if (context != null) { try diff --git a/src/NuGetGallery/App_Start/AppActivator.cs b/src/NuGetGallery/App_Start/AppActivator.cs index 1e6d8cfa2d..1db3357282 100644 --- a/src/NuGetGallery/App_Start/AppActivator.cs +++ b/src/NuGetGallery/App_Start/AppActivator.cs @@ -252,6 +252,7 @@ private static void AppPostStart(IAppConfiguration configuration) Routes.RegisterRoutes(RouteTable.Routes, configuration.FeedOnlyMode, configuration.AdminPanelEnabled); AreaRegistration.RegisterAllAreas(); + GlobalFilters.Filters.Add(new SendErrorsToTelemetryAttribute { View = "~/Views/Errors/InternalError.cshtml" }); GlobalFilters.Filters.Add(new ReadOnlyModeErrorFilter()); GlobalFilters.Filters.Add(new AntiForgeryErrorFilter()); GlobalFilters.Filters.Add(new UserDeletedErrorFilter());