Skip to content

Commit a70b5c1

Browse files
Improve the logging error handling in our ServiceHub MEF creation (#80124)
This improves some logging and handling in error situations for our ServiceHub MEF creation. **Review commit-at-a-time.**
2 parents 77b605c + e992e64 commit a70b5c1

File tree

6 files changed

+64
-28
lines changed

6 files changed

+64
-28
lines changed

src/Deployment/Properties/launchSettings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"executablePath": "$(DevEnvDir)devenv.exe",
1111
"commandLineArgs": "/rootsuffix $(VSSDKTargetPlatformRegRootSuffix) /log",
1212
"environmentVariables": {
13-
"SERVICEHUBDEBUGHOSTONSTARTUP": "All"
13+
"SERVICEHUBDEBUGHOSTONSTARTUP": "ServiceHub.RoslynCodeAnalysisService.exe"
1414
}
1515
},
1616
"Visual Studio Extension (with Native Debugging)": {

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServerExportProviderBuilder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ public static async Task<ExportProvider> CreateExportProviderAsync(
7474
return exportProvider;
7575
}
7676

77-
protected override void LogError(string message)
78-
=> _logger.LogError(message);
77+
protected override void LogError(string message, Exception exception)
78+
=> _logger.LogError(exception, message);
7979

8080
protected override void LogTrace(string message)
8181
=> _logger.LogTrace(message);

src/VisualStudio/Core/Def/ErrorReporting/VisualStudioErrorReportingService.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,17 @@ public void ShowGlobalErrorInfo(string message, TelemetryFeatureName featureName
4949
LogGlobalErrorToActivityLog(message, stackTrace);
5050
_infoBar.ShowInfoBarMessageFromAnyThread(message, items);
5151

52-
Logger.Log(FunctionId.VS_ErrorReportingService_ShowGlobalErrorInfo, KeyValueLogMessage.Create(LogType.UserAction, static (m, args) =>
52+
Logger.Log(FunctionId.VS_ErrorReportingService_ShowGlobalErrorInfo, KeyValueLogMessage.Create(LogType.UserAction, (m, args) =>
5353
{
5454
var (message, featureName) = args;
5555
m["Message"] = message;
5656
m["FeatureName"] = featureName.ToString();
57+
58+
if (exception is not null)
59+
{
60+
m["ExceptionType"] = exception.GetType().Name;
61+
m["ExceptionStackTrace"] = stackTrace;
62+
}
5763
}, (message, featureName)));
5864
}
5965

src/Workspaces/Remote/Core/ExportProviderBuilder.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ internal abstract class ExportProviderBuilder(
3232
protected string CacheDirectory { get; } = cacheDirectory;
3333
protected string CatalogPrefix { get; } = catalogPrefix;
3434

35-
protected abstract void LogError(string message);
35+
protected abstract void LogError(string message, Exception exception);
3636
protected abstract void LogTrace(string message);
3737

3838
protected virtual async Task<ExportProvider> CreateExportProviderAsync(CancellationToken cancellationToken)
@@ -69,7 +69,7 @@ private async Task<IExportProviderFactory> GetCompositionConfigurationAsync(Canc
6969
catch (Exception ex)
7070
{
7171
// Log the error, and move on to recover by recreating the MEF composition.
72-
LogError($"Loading cached MEF composition failed: {ex}");
72+
LogError("Loading cached MEF composition failed", ex);
7373
}
7474

7575
LogTrace($"Composing MEF catalog using:{Environment.NewLine}{string.Join($" {Environment.NewLine}", AssemblyPaths)}.");
@@ -165,7 +165,7 @@ protected virtual async Task WriteCompositionCacheAsync(string compositionCacheF
165165
}
166166
catch (Exception ex)
167167
{
168-
LogError($"Failed to save MEF cache: {ex}");
168+
LogError("Failed to save MEF cache", ex);
169169
}
170170
}
171171

@@ -191,11 +191,11 @@ private bool CheckForAndReportCompositionErrors(CompositionConfiguration configu
191191
foreach (var exception in catalog.DiscoveredParts.DiscoveryErrors)
192192
{
193193
hasErrors = true;
194-
LogError($"Encountered exception in the MEF composition: {exception.Message}");
194+
LogError("Encountered exception in the MEF composition", exception);
195195
}
196196

197197
// Verify that we have exactly the MEF errors that we expect. If we have less or more this needs to be updated to assert the expected behavior.
198-
var erroredParts = configuration.CompositionErrors.FirstOrDefault()?.SelectMany(error => error.Parts).Select(part => part.Definition.Type.Name) ?? [];
198+
var erroredParts = configuration.CompositionErrors.SelectMany(c => c).SelectMany(error => error.Parts).Select(part => part.Definition.Type.Name);
199199

200200
if (ContainsUnexpectedErrors(erroredParts))
201201
{
@@ -208,7 +208,7 @@ private bool CheckForAndReportCompositionErrors(CompositionConfiguration configu
208208
catch (CompositionFailedException ex)
209209
{
210210
// The ToString for the composition failed exception doesn't output a nice set of errors by default, so log it separately
211-
LogError($"Encountered errors in the MEF composition: {ex.Message}{Environment.NewLine}{ex.ErrorsAsString}");
211+
LogError($"Encountered errors in the MEF composition:{Environment.NewLine}{ex.ErrorsAsString}", ex);
212212
}
213213
}
214214

src/Workspaces/Remote/ServiceHub/Host/RemoteExportProviderBuilder.cs

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System;
66
using System.Collections.Generic;
77
using System.Collections.Immutable;
8+
using System.Diagnostics;
89
using System.IO;
910
using System.Linq;
1011
using System.Reflection;
@@ -35,42 +36,61 @@ internal static ExportProvider ExportProvider
3536
=> s_instance ?? throw new InvalidOperationException($"Default export provider not initialized. Call {nameof(InitializeAsync)} first.");
3637

3738
private StringBuilder? _errorMessages;
39+
private readonly TraceSource _traceLogger;
3840

3941
private RemoteExportProviderBuilder(
4042
ImmutableArray<string> assemblyPaths,
4143
Resolver resolver,
4244
string cacheDirectory,
43-
string catalogPrefix)
45+
string catalogPrefix,
46+
TraceSource traceLogger)
4447
: base(assemblyPaths, resolver, cacheDirectory, catalogPrefix)
4548
{
49+
_traceLogger = traceLogger;
4650
}
4751

48-
public static async Task<string?> InitializeAsync(string localSettingsDirectory, CancellationToken cancellationToken)
52+
public static async Task<string?> InitializeAsync(string localSettingsDirectory, TraceSource traceLogger, CancellationToken cancellationToken)
4953
{
50-
var assemblyPaths = RemoteHostAssemblyNames
51-
.Select(static assemblyName => MefHostServicesHelpers.TryFindNearbyAssemblyLocation(assemblyName))
52-
.WhereNotNull()
53-
.AsImmutable();
54+
var assemblyPaths = ImmutableArray.CreateBuilder<string>(initialCapacity: RemoteHostAssemblyNames.Length);
55+
56+
foreach (var assemblyName in RemoteHostAssemblyNames)
57+
{
58+
var assemblyPath = MefHostServicesHelpers.TryFindNearbyAssemblyLocation(assemblyName);
59+
if (assemblyPath != null)
60+
{
61+
assemblyPaths.Add(assemblyPath);
62+
traceLogger.TraceInformation($"Located {assemblyPath} for the MEF composition.");
63+
}
64+
else
65+
{
66+
traceLogger.TraceEvent(TraceEventType.Warning, 0, $"Could not find assembly '{assemblyName}' near '{typeof(MefHostServicesHelpers).Assembly.Location}'");
67+
}
68+
}
5469

5570
var builder = new RemoteExportProviderBuilder(
56-
assemblyPaths: assemblyPaths,
71+
assemblyPaths: assemblyPaths.ToImmutable(),
5772
resolver: new Resolver(SimpleAssemblyLoader.Instance),
5873
cacheDirectory: Path.Combine(localSettingsDirectory, "Roslyn", "RemoteHost", "Cache"),
59-
catalogPrefix: "RoslynRemoteHost");
74+
catalogPrefix: "RoslynRemoteHost",
75+
traceLogger);
6076

6177
s_instance = await builder.CreateExportProviderAsync(cancellationToken).ConfigureAwait(false);
6278

6379
return builder._errorMessages?.ToString();
6480
}
6581

66-
protected override void LogError(string message)
82+
protected override void LogError(string message, Exception exception)
6783
{
84+
// We'll log just the message to _errorMessages (since that gets displayed to the user in a gold bar), but we'll log the
85+
// full exception to the logger.
6886
_errorMessages ??= new StringBuilder();
69-
_errorMessages.AppendLine(message);
87+
_errorMessages.AppendLine($"{message}: {exception.Message}");
88+
_traceLogger.TraceEvent(TraceEventType.Error, 0, $"{message}: {exception}");
7089
}
7190

7291
protected override void LogTrace(string message)
7392
{
93+
_traceLogger.TraceEvent(TraceEventType.Information, 0, message);
7494
}
7595

7696
protected override bool ContainsUnexpectedErrors(IEnumerable<string> erroredParts)

src/Workspaces/Remote/ServiceHub/Services/Initialization/RemoteInitializationService.cs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.Diagnostics;
67
using System.Threading;
78
using System.Threading.Tasks;
89
using Microsoft.CodeAnalysis.Host;
9-
using Roslyn.Utilities;
1010

1111
namespace Microsoft.CodeAnalysis.Remote;
1212

@@ -24,16 +24,26 @@ protected override IRemoteInitializationService CreateService(in ServiceConstruc
2424
{
2525
// Performed before RunServiceAsync to ensure that the export provider is initialized before the RemoteWorkspaceManager is created
2626
// as part of the RunServiceAsync call.
27-
var errorMessage = await RemoteExportProviderBuilder.InitializeAsync(localSettingsDirectory, cancellationToken).ConfigureAwait(false);
27+
var errorMessage = await RemoteExportProviderBuilder.InitializeAsync(localSettingsDirectory, TraceLogger, cancellationToken).ConfigureAwait(false);
2828

29-
var processId = await RunServiceAsync(cancellationToken =>
29+
try
3030
{
31-
var service = (RemoteWorkspaceConfigurationService)GetWorkspaceServices().GetRequiredService<IWorkspaceConfigurationService>();
32-
service.InitializeOptions(options);
31+
var processId = await RunServiceAsync(cancellationToken =>
32+
{
33+
var service = (RemoteWorkspaceConfigurationService)GetWorkspaceServices().GetRequiredService<IWorkspaceConfigurationService>();
34+
service.InitializeOptions(options);
3335

34-
return ValueTask.FromResult(Process.GetCurrentProcess().Id);
35-
}, cancellationToken).ConfigureAwait(false);
36+
return ValueTask.FromResult(Process.GetCurrentProcess().Id);
37+
}, cancellationToken).ConfigureAwait(false);
3638

37-
return (processId, errorMessage);
39+
return (processId, errorMessage);
40+
}
41+
catch (Exception ex) when (errorMessage != null)
42+
{
43+
// We want to throw the exception but also include the message from the MEF creation
44+
throw new AggregateException(
45+
$"Error from {nameof(RemoteExportProviderBuilder)}: {errorMessage}",
46+
ex);
47+
}
3848
}
3949
}

0 commit comments

Comments
 (0)