Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nuget.exe push needs a helpful error message for timeout #442

Merged
merged 1 commit into from
Apr 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/PushCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ await PushRunner.Run(
NoSymbols,
Console);
}
catch (TaskCanceledException ex)
{
string timeoutMessage = LocalizedResourceManager.GetString(nameof(NuGetResources.PushCommandTimeoutError));
throw new AggregateException(ex, new Exception(timeoutMessage));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect this to throw a more specific exception type. This works because it is just caught and written out to the console?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the console programs catch all exceptions and print out their message as an error. I used AggregateException so that the original TaskCanceledException can be preserved to show its message and optionally its callstack (with verbose output is turned on). And then I combined it with a generic Exception so that a custom message can be displayed.

But if the callstack of the original TaskCanceledException doesn't matter, I can just throw a new specific type of exception here.


In reply to: 58240759 [](ancestors = 58240759)

}
catch (Exception ex)
{
if (ex is HttpRequestException && ex.InnerException is WebException)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/NuGetResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6076,4 +6076,7 @@ Oluşturma sırasında NuGet'in paketleri indirmesini önlemek için, Visual Stu
<data name="Error_UnableToLocateRestoreTarget" xml:space="preserve">
<value>The folder '{0}' does not contain an msbuild solution, packages.config, or project.json file to restore.</value>
</data>
<data name="PushCommandTimeoutError" xml:space="preserve">
<value>Pushing took too long. You can change the default timeout of 300 seconds by using the -Timeout &lt;seconds&gt; option with the push command.</value>
</data>
</root>
28 changes: 18 additions & 10 deletions src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PushCommand.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using System.Threading.Tasks;
using Microsoft.Dnx.Runtime.Common.CommandLine;
using NuGet.Commands;
using NuGet.Configuration;
Expand Down Expand Up @@ -66,16 +67,23 @@ public static void Register(CommandLineApplication app, Func<ILogger> getLogger)

PackageSourceProvider sourceProvider = new PackageSourceProvider(XPlatUtility.CreateDefaultSettings());

await PushRunner.Run(
sourceProvider.Settings,
sourceProvider,
packagePath,
sourcePath,
apiKeyValue,
timeoutSeconds,
disableBufferingValue,
noSymbolsValue,
getLogger());
try
{
await PushRunner.Run(
sourceProvider.Settings,
sourceProvider,
packagePath,
sourcePath,
apiKeyValue,
timeoutSeconds,
disableBufferingValue,
noSymbolsValue,
getLogger());
}
catch (TaskCanceledException ex)
{
throw new AggregateException(ex, new Exception(Strings.Push_Timeout_Error));
}

return 0;
});
Expand Down
9 changes: 9 additions & 0 deletions src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -293,4 +293,7 @@
<data name="Exclude_Description" xml:space="preserve">
<value>Specifies one or more wildcard patterns to exclude when creating a package.</value>
</data>
<data name="Push_Timeout_Error" xml:space="preserve">
<value>Pushing took too long. You can change the default timeout of 300 seconds by using the --timeout &lt;seconds&gt; option with the push command.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,35 @@ public void PushCommand_PushToServerNoSymbols()
}
}

[Fact]
public void PushCommand_PushTimeoutErrorMessage()
{
using (TestDirectory packageDirectory = TestFileSystemUtility.CreateRandomTestFolder())
using (MockServer server = new MockServer())
{
// Arrange
string packageFileName = Util.CreateTestPackage("testPackage1", "1.1.0", packageDirectory);
server.Get.Add("/push", r => "OK");
server.Put.Add("/push", r =>
{
System.Threading.Thread.Sleep(2000);
return HttpStatusCode.Created;
});
server.Start();

// Act
CommandRunnerResult result = CommandRunner.Run(
Util.GetNuGetExePath(),
Directory.GetCurrentDirectory(),
$"push {packageFileName} -Source {server.Uri}push -Timeout 1",
waitForExit: true);

// Assert
Assert.Equal(1, result.Item1);
Assert.Contains("took too long", result.Item3);
}
}

// Tests that push command can follow redirection correctly.
[Fact]
public void PushCommand_PushToServerFollowRedirection()
Expand Down