Skip to content

Commit

Permalink
Plugins: propagate plugin exception (#2838)
Browse files Browse the repository at this point in the history 
Resolve NuGet/Home#8057.
  • Loading branch information
@dtivel
dtivel committed May 3, 2019
1 parent c8fc660 commit f34e467
Show file tree
Hide file tree
Showing 20 changed files with 501 additions and 375 deletions.
2 changes: 1 addition & 1 deletion NuGet.sln
View file
Expand Up @@ -624,7 +624,7 @@ Global
{50E33DA2-AF14-486D-81B8-BD8409744A38} = {08F523EC-3C2A-4A00-A54C-2E54C5AC856B}
{23725516-0A6E-48F9-A148-B5E3FA515B13} = {00CE0FBE-DA5A-4AD8-B0D6-04ECE6B04F0D}
{5039C2F9-CAD9-4162-849C-5EF7E3CE03D9} = {01BC4531-1E25-48D7-A8FD-A47D6FEC47FB}
{3DB4EDE3-B57B-498C-9EA9-A8BF8D9D4B2B} = {01BC4531-1E25-48D7-A8FD-A47D6FEC47FB}
{3DB4EDE3-B57B-498C-9EA9-A8BF8D9D4B2B} = {7323F93B-D141-4001-BB9E-7AF14031143E}
{6157BF46-D371-4126-A7F1-ED6E23F77ACD} = {01BC4531-1E25-48D7-A8FD-A47D6FEC47FB}
{9EA84487-C70C-420C-9674-75CF19F43757} = {01BC4531-1E25-48D7-A8FD-A47D6FEC47FB}
{F7837EEB-1B49-482F-8722-EB35BA0937CC} = {01BC4531-1E25-48D7-A8FD-A47D6FEC47FB}
Expand Down
4 changes: 2 additions & 2 deletions runTests.ps1
View file
Expand Up @@ -87,9 +87,9 @@ Invoke-BuildStep 'Cleaning package cache' {
-skip:(-not $CI) `
-ev +BuildErrors

Invoke-BuildStep 'Running /t:RestoreVS15' {
Invoke-BuildStep 'Running /t:RestoreVS' {

& $MSBuildExe build\build.proj /t:RestoreVS15 /p:Configuration=$Configuration /p:ReleaseLabel=$ReleaseLabel /p:BuildNumber=$BuildNumber /v:m /m:1
& $MSBuildExe build\build.proj /t:RestoreVS /p:Configuration=$Configuration /p:ReleaseLabel=$ReleaseLabel /p:BuildNumber=$BuildNumber /v:m /m:1

if (-not $?)
{
Expand Down
6 changes: 4 additions & 2 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/Command.cs
View file
Expand Up @@ -15,6 +15,7 @@
using NuGet.Credentials;
using NuGet.Protocol;
using NuGet.Protocol.Core.Types;
using NuGet.Protocol.Plugins;

namespace NuGet.CommandLine
{
Expand Down Expand Up @@ -78,7 +79,8 @@ public string CurrentDirectory

protected internal CoreV2.NuGet.IPackageRepositoryFactory RepositoryFactory { get; set; }

private Lazy<MsBuildToolset> MsBuildToolset {
private Lazy<MsBuildToolset> MsBuildToolset
{
get
{
if (_defaultMsBuildToolset == null)
Expand Down Expand Up @@ -211,7 +213,7 @@ private async Task<IEnumerable<ICredentialProvider>> GetCredentialProvidersAsync
var pluginProviders = new PluginCredentialProviderBuilder(extensionLocator, Settings, Console)
.BuildAll(Verbosity.ToString())
.ToList();
var securePluginProviders = await (new SecurePluginCredentialProviderBuilder(pluginManager: PluginManager.Instance, canShowDialog: true, logger: Console)).BuildAllAsync();
var securePluginProviders = await (new SecurePluginCredentialProviderBuilder(PluginManager.Instance, canShowDialog: true, logger: Console)).BuildAllAsync();

providers.Add(new CredentialProviderAdapter(new SettingsCredentialProvider(SourceProvider, Console)));
providers.AddRange(securePluginProviders);
Expand Down
7 changes: 2 additions & 5 deletions ...uGet.PackageManagement.PowerShellCmdlets/NuGet.PackageManagement.PowerShellCmdlets.csproj
View file
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'README.md'))\build\common.legacy.props" />
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
Expand Down Expand Up @@ -77,10 +77,7 @@
<ItemGroup>
<ProjectReference Include="$(NuGetCoreSrcDirectory)NuGet.Resolver\NuGet.Resolver.csproj" />
<ProjectReference Include="..\NuGet.PackageManagement.VisualStudio\NuGet.PackageManagement.VisualStudio.csproj" />
<ProjectReference Include="..\NuGet.VisualStudio\NuGet.VisualStudio.csproj">
<Project>{e5556bc6-a7fd-4d8e-8a7d-7648df1d7471}</Project>
<Name>NuGet.VisualStudio</Name>
</ProjectReference>
<ProjectReference Include="..\NuGet.VisualStudio\NuGet.VisualStudio.csproj" />
<ProjectReference Include="..\NuGet.VisualStudio.Common\NuGet.VisualStudio.Common.csproj" />
</ItemGroup>
<ItemGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/DefaultVSCredentialServiceProvider.cs
View file
Expand Up @@ -11,7 +11,7 @@
using NuGet.Common;
using NuGet.Credentials;
using NuGet.ProjectManagement;
using NuGet.Protocol.Core.Types;
using NuGet.Protocol.Plugins;
using NuGet.VisualStudio;
using IAsyncServiceProvider = Microsoft.VisualStudio.Shell.IAsyncServiceProvider;

Expand Down Expand Up @@ -150,4 +150,4 @@ private void LogCredentialProviderError(Exception exception, string failureMessa
exception);
}
}
}
}
5 changes: 2 additions & 3 deletions src/NuGet.Core/NuGet.Credentials/DefaultCredentialServiceUtility.cs
View file
Expand Up @@ -8,7 +8,7 @@
using NuGet.Common;
using NuGet.Configuration;
using NuGet.Protocol;
using NuGet.Protocol.Core.Types;
using NuGet.Protocol.Plugins;

namespace NuGet.Credentials
{
Expand Down Expand Up @@ -45,6 +45,5 @@ private static async Task<IEnumerable<ICredentialProvider>> GetCredentialProvide
}
return providers;
}

}
}
}
42 changes: 23 additions & 19 deletions src/NuGet.Core/NuGet.Credentials/SecurePluginCredentialProvider.cs
View file
Expand Up @@ -2,7 +2,6 @@
// 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 System.Globalization;
using System.Linq;
using System.Net;
Expand Down Expand Up @@ -41,7 +40,6 @@ public sealed class SecurePluginCredentialProvider : ICredentialProvider
// We use this to avoid needlessly instantiating plugins if they don't support authentication.
private bool _isAnAuthenticationPlugin = true;


/// <summary>
/// Create a credential provider based on provided plugin
/// </summary>
Expand Down Expand Up @@ -89,48 +87,54 @@ public async Task<CredentialResponse> GetAsync(Uri uri, IWebProxy proxy, Credent
return taskResponse;
}

var creationResult = await _pluginManager.TryGetSourceAgnosticPluginAsync(_discoveredPlugin, OperationClaim.Authentication, cancellationToken);
Tuple<bool, PluginCreationResult> result = await _pluginManager.TryGetSourceAgnosticPluginAsync(_discoveredPlugin, OperationClaim.Authentication, cancellationToken);

bool wasSomethingCreated = result.Item1;

if (creationResult.Item1) // status of the source creation
if (wasSomethingCreated)
{
var plugin = creationResult.Item2; // plugin creation result
if (!string.IsNullOrEmpty(plugin.Message))
PluginCreationResult creationResult = result.Item2;

if (!string.IsNullOrEmpty(creationResult.Message))
{
// There is a potential here for double logging as the CredentialService itself catches the exceptions and tries to log it.
// In reality the logger in the Credential Service will be null because the first request always comes from a resource provider (ServiceIndex provider)
_logger.LogError(plugin.Message);
// In reality the logger in the Credential Service will be null because the first request always comes from a resource provider (ServiceIndex provider).
_logger.LogError(creationResult.Message);

if (creationResult.Exception != null)
{
_logger.LogDebug(creationResult.Exception.ToString());
}
_isAnAuthenticationPlugin = false;
throw new PluginException(plugin.Message); // Throwing here will block authentication and ensure that the complete operation fails
throw new PluginException(creationResult.Message, creationResult.Exception); // Throwing here will block authentication and ensure that the complete operation fails.
}

_isAnAuthenticationPlugin = plugin.Claims.Contains(OperationClaim.Authentication);
_isAnAuthenticationPlugin = creationResult.Claims.Contains(OperationClaim.Authentication);

if (_isAnAuthenticationPlugin)
{
AddOrUpdateLogger(plugin.Plugin);
await SetPluginLogLevelAsync(plugin, _logger, cancellationToken);
AddOrUpdateLogger(creationResult.Plugin);
await SetPluginLogLevelAsync(creationResult, _logger, cancellationToken);

if (proxy != null)
{
await SetProxyCredentialsToPlugin(uri, proxy, plugin, cancellationToken);
await SetProxyCredentialsToPlugin(uri, proxy, creationResult, cancellationToken);
}

var request = new GetAuthenticationCredentialsRequest(uri, isRetry, nonInteractive, _canShowDialog);
var credentialResponse = await plugin.Plugin.Connection.SendRequestAndReceiveResponseAsync<GetAuthenticationCredentialsRequest, GetAuthenticationCredentialsResponse>(
var credentialResponse = await creationResult.Plugin.Connection.SendRequestAndReceiveResponseAsync<GetAuthenticationCredentialsRequest, GetAuthenticationCredentialsResponse>(
MessageMethod.GetAuthenticationCredentials,
request,
cancellationToken);
if (credentialResponse.ResponseCode == MessageResponseCode.NotFound && nonInteractive)
{
_logger.LogWarning(
string.Format(
CultureInfo.CurrentCulture,
Resources.SecurePluginWarning_UseInteractiveOption)
);
CultureInfo.CurrentCulture,
Resources.SecurePluginWarning_UseInteractiveOption));
}

taskResponse = GetAuthenticationCredentialsResponseToCredentialResponse(credentialResponse);

}
}
else
Expand Down Expand Up @@ -214,4 +218,4 @@ private static CredentialResponse GetAuthenticationCredentialsResponseToCredenti
return taskResponse;
}
}
}
}
26 changes: 25 additions & 1 deletion src/NuGet.Core/NuGet.Protocol/Plugins/PluginCreationResult.cs
View file
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand All @@ -21,6 +21,11 @@ public sealed class PluginCreationResult
/// </summary>
public string Message { get; }

/// <summary>
/// Gets the exception caught. May be <c>null</c>.
/// </summary>
public Exception Exception { get; }

/// <summary>
/// Gets a plugin.
/// </summary>
Expand Down Expand Up @@ -77,5 +82,24 @@ public PluginCreationResult(string message)

Message = message;
}

/// <summary>
/// Instantiates a new <see cref="PluginCreationResult" /> class.
/// </summary>
/// <param name="message">A message why a plugin could not be created.</param>
/// <param name="exception">An exception.</param>
/// <exception cref="ArgumentException">Thrown if <paramref name="message" />
/// is either <c>null</c> or an empty string.</exception>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="exception" /> is <c>null</c>.</exception>
public PluginCreationResult(string message, Exception exception)
: this(message)
{
if (exception == null)
{
throw new ArgumentNullException(nameof(exception));
}

Exception = exception;
}
}
}
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Protocol/Plugins/PluginFactory.cs
View file
Expand Up @@ -391,7 +391,7 @@ private void OnPluginProcessExited(object sender, EventArgs e, string pluginId,
}
}

private void SendCloseRequest(IPlugin plugin)
private static void SendCloseRequest(IPlugin plugin)
{
var message = plugin.Connection.MessageDispatcher.CreateMessage(
MessageType.Request,
Expand Down
69 changes: 32 additions & 37 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs
View file
Expand Up @@ -13,19 +13,19 @@
using NuGet.Common;
using NuGet.Configuration;
using NuGet.Packaging;
using NuGet.Protocol.Plugins;
using NuGet.Protocol.Core.Types;
using NuGet.Shared;

namespace NuGet.Protocol.Core.Types
namespace NuGet.Protocol.Plugins
{
/// <summary>
/// A plugin manager. This manages all the live plugins and their operation claims.
/// Invoked in by both the credential provider and the PluginResourceProvider
/// </summary>
public sealed class PluginManager : IPluginManager, IDisposable
{
private static readonly Lazy<IPluginManager> Lazy = new Lazy<IPluginManager>(() => new PluginManager());
public static IPluginManager Instance => Lazy.Value;
private static readonly Lazy<IPluginManager> _lazy = new Lazy<IPluginManager>(() => new PluginManager());
public static IPluginManager Instance => _lazy.Value;

private ConnectionOptions _connectionOptions;
private Lazy<IPluginDiscoverer> _discoverer;
Expand All @@ -36,7 +36,7 @@ public sealed class PluginManager : IPluginManager, IDisposable
private string _rawPluginPaths;

private static Lazy<int> _currentProcessId = new Lazy<int>(GetCurrentProcessId);
private Lazy<string> _pluginsCacheDirectory = new Lazy<string>(() => SettingsUtility.GetPluginsCacheFolder());
private Lazy<string> _pluginsCacheDirectoryPath;

/// <summary>
/// Gets an environment variable reader.
Expand All @@ -49,29 +49,21 @@ private PluginManager()
Initialize(
new EnvironmentVariableWrapper(),
new Lazy<IPluginDiscoverer>(InitializeDiscoverer),
(TimeSpan idleTimeout) => new PluginFactory(idleTimeout));
(TimeSpan idleTimeout) => new PluginFactory(idleTimeout),
new Lazy<string>(() => SettingsUtility.GetPluginsCacheFolder()));
}

/// <summary>
/// Creates a new plugin manager
/// </summary>
/// <remarks>This is public to facilitate unit testing. This should not be called from product code</remarks>
/// <param name="reader">An environment variable reader.</param>
/// <param name="pluginDiscoverer">A lazy plugin discoverer.</param>
/// <param name="pluginFactoryCreator">A plugin factory creator.</param>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="reader" /> is <c>null</c>.</exception>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="pluginDiscoverer" />
/// is <c>null</c>.</exception>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="pluginFactoryCreator" />
/// is <c>null</c>.</exception>
public PluginManager(IEnvironmentVariableReader reader,
internal PluginManager(
IEnvironmentVariableReader reader,
Lazy<IPluginDiscoverer> pluginDiscoverer,
Func<TimeSpan, IPluginFactory> pluginFactoryCreator)
Func<TimeSpan, IPluginFactory> pluginFactoryCreator,
Lazy<string> pluginsCacheDirectoryPath)
{
Initialize(
reader,
pluginDiscoverer,
pluginFactoryCreator);
pluginFactoryCreator,
pluginsCacheDirectoryPath);
}

/// <summary>
Expand Down Expand Up @@ -202,7 +194,7 @@ public async Task<IEnumerable<PluginDiscoveryResult>> FindAvailablePluginsAsync(
CancellationToken cancellationToken)
{
PluginCreationResult pluginCreationResult = null;
var cacheEntry = new PluginCacheEntry(_pluginsCacheDirectory.Value, result.PluginFile.Path, requestKey.PackageSourceRepository);
var cacheEntry = new PluginCacheEntry(_pluginsCacheDirectoryPath.Value, result.PluginFile.Path, requestKey.PackageSourceRepository);

return await ConcurrencyUtilities.ExecuteWithFileLockedAsync(
cacheEntry.CacheFileName,
Expand All @@ -225,13 +217,13 @@ public async Task<IEnumerable<PluginDiscoveryResult>> FindAvailablePluginsAsync(
// We still make the GetOperationClaims call even if we have the operation claims cached. This is a way to self-update the cache.
var operationClaims = await _pluginOperationClaims.GetOrAdd(
requestKey,
key => new Lazy<Task<IReadOnlyList<OperationClaim>>>(() =>
GetPluginOperationClaimsAsync(
plugin,
packageSourceRepository,
serviceIndex,
cancellationToken))).Value;
requestKey,
key => new Lazy<Task<IReadOnlyList<OperationClaim>>>(() =>
GetPluginOperationClaimsAsync(
plugin,
packageSourceRepository,
serviceIndex,
cancellationToken))).Value;
if (!EqualityUtility.SequenceEqualWithNullCheck(operationClaims, cacheEntry.OperationClaims))
{
Expand All @@ -251,12 +243,12 @@ public async Task<IEnumerable<PluginDiscoveryResult>> FindAvailablePluginsAsync(
}
catch (Exception e)
{
// Have a clear error message when the plugin creation fails.
pluginCreationResult = new PluginCreationResult(
string.Format(CultureInfo.CurrentCulture,
Strings.Plugin_ProblemStartingPlugin,
result.PluginFile.Path,
e.Message));
Strings.Plugin_ProblemStartingPlugin,
result.PluginFile.Path,
e.Message),
e);
}
}
return new Tuple<bool, PluginCreationResult>(pluginCreationResult != null, pluginCreationResult);
Expand All @@ -268,9 +260,9 @@ public async Task<IEnumerable<PluginDiscoveryResult>> FindAvailablePluginsAsync(
private async Task<Lazy<IPluginMulticlientUtilities>> PerformOneTimePluginInitializationAsync(IPlugin plugin, CancellationToken cancellationToken)
{
var utilities = _pluginUtilities.GetOrAdd(
plugin.Id,
path => new Lazy<IPluginMulticlientUtilities>(
() => new PluginMulticlientUtilities()));
plugin.Id,
path => new Lazy<IPluginMulticlientUtilities>(
() => new PluginMulticlientUtilities()));

await utilities.Value.DoOncePerPluginLifetimeAsync(
MessageMethod.MonitorNuGetProcessExit.ToString(),
Expand All @@ -284,15 +276,18 @@ private async Task<Lazy<IPluginMulticlientUtilities>> PerformOneTimePluginInitia
MessageMethod.Initialize.ToString(),
() => InitializePluginAsync(plugin, _connectionOptions.RequestTimeout, cancellationToken),
cancellationToken);

return utilities;
}

private void Initialize(IEnvironmentVariableReader reader,
Lazy<IPluginDiscoverer> pluginDiscoverer,
Func<TimeSpan, IPluginFactory> pluginFactoryCreator)
Func<TimeSpan, IPluginFactory> pluginFactoryCreator,
Lazy<string> pluginsCacheDirectoryPath)
{
EnvironmentVariableReader = reader ?? throw new ArgumentNullException(nameof(reader));
_discoverer = pluginDiscoverer ?? throw new ArgumentNullException(nameof(pluginDiscoverer));
_pluginsCacheDirectoryPath = pluginsCacheDirectoryPath ?? throw new ArgumentNullException(nameof(pluginsCacheDirectoryPath));

if (pluginFactoryCreator == null)
{
Expand Down

0 comments on commit f34e467

Please sign in to comment.