Skip to content

Adjust and document exit codes #4787

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

Merged
merged 10 commits into from
Jun 7, 2022
Merged

Conversation

JanKrivanek
Copy link
Member

Problem

Fixes #3186
Fixes #4253

Solution

  • Used well know exit codes where applicable
  • Defined user exit codes in 100-106 for others
  • Reviewed usages of NewCommandStatus, fixed some of the controversial usages
  • Documented our exit codes
  • Created aka.ms alias link pointing to our help (once merged)
  • Added spitting of help link in case of nonzero exit

Checks: N/A

  • Added unit tests
  • Added #nullable enable to all the modified files ?

@JanKrivanek JanKrivanek requested a review from vlada-shubina May 25, 2022 17:21
@JanKrivanek JanKrivanek requested review from a team and baronfel as code owners May 25, 2022 17:21
@JanKrivanek JanKrivanek mentioned this pull request May 25, 2022
2 tasks
Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

please consider also parsing errors in exit codes, feels like 102 and 103 may be merged.

@vlada-shubina
Copy link
Member

private async Task DisplayInstallResultAsync(string packageToInstall, InstallerOperationResult result, ParseResult parseResult, CancellationToken cancellationToken)
{
if (string.IsNullOrWhiteSpace(packageToInstall))
{
throw new ArgumentException(nameof(packageToInstall));
}
_ = result ?? throw new ArgumentNullException(nameof(result));
cancellationToken.ThrowIfCancellationRequested();
if (result.Success)
{
IEnumerable<ITemplateInfo> templates = await _templatePackageManager.GetTemplatesAsync(result.TemplatePackage, cancellationToken).ConfigureAwait(false);
if (templates.Any())
{
Reporter.Output.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Info_Success,
result.TemplatePackage.DisplayName));
TemplateGroupDisplay.DisplayTemplateList(
_engineEnvironmentSettings,
templates,
new TabularOutputSettings(_engineEnvironmentSettings.Environment),
reporter: Reporter.Output);
}
else
{
Reporter.Output.WriteLine(string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Warning_No_Templates_In_Package,
result.TemplatePackage.DisplayName));
}
}
else
{
switch (result.Error)
{
case InstallerErrorCode.InvalidSource:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_InvalidNuGetFeeds,
packageToInstall,
result.ErrorMessage).Bold().Red());
break;
case InstallerErrorCode.PackageNotFound:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_PackageNotFound,
packageToInstall).Bold().Red());
break;
case InstallerErrorCode.DownloadFailed:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_DownloadFailed,
packageToInstall).Bold().Red());
break;
case InstallerErrorCode.UnsupportedRequest:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_UnsupportedRequest,
packageToInstall).Bold().Red());
break;
case InstallerErrorCode.AlreadyInstalled:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_AlreadyInstalled,
packageToInstall).Bold().Red());
Reporter.Error.WriteLine(string.Format(LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_AlreadyInstalled_Hint, InstallCommand.ForceOption.Aliases.First()));
Reporter.Error.WriteCommand(Example.For<InstallCommand>(parseResult).WithArgument(BaseInstallCommand.NameArgument, packageToInstall).WithOption(BaseInstallCommand.ForceOption));
break;
case InstallerErrorCode.UpdateUninstallFailed:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_UninstallFailed,
packageToInstall).Bold().Red());
break;
case InstallerErrorCode.InvalidPackage:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_InvalidPackage,
packageToInstall).Bold().Red());
break;
case InstallerErrorCode.GenericError:
default:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_lnstall_Error_GenericError,
packageToInstall).Bold().Red());
break;
}
}
}
private void HandleUpdateCheckErrors(InstallerOperationResult result, bool ignoreLocalPackageNotFound)
{
switch (result.Error)
{
case InstallerErrorCode.InvalidSource:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_Update_Error_InvalidNuGetFeeds,
result.TemplatePackage.DisplayName).Bold().Red());
break;
case InstallerErrorCode.PackageNotFound:
if (!ignoreLocalPackageNotFound || !result.TemplatePackage.IsLocalPackage)
{
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_Update_Error_PackageNotFound,
result.TemplatePackage.DisplayName).Bold().Red());
}
break;
case InstallerErrorCode.UnsupportedRequest:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_Update_Error_PackageNotSupported,
result.TemplatePackage.DisplayName).Bold().Red());
break;
case InstallerErrorCode.GenericError:
default:
Reporter.Error.WriteLine(
string.Format(
LocalizableStrings.TemplatePackageCoordinator_Update_Error_GenericError,
result.TemplatePackage.DisplayName,
result.ErrorMessage).Bold().Red());
break;
}
}

Template package failure may be one of the following. Just wondering if different codes make sense here.
E.g. in case download problem, it might be retried; however if package is not found or already installed retry won't help.

@JanKrivanek JanKrivanek requested a review from vlada-shubina May 30, 2022 17:01
@JanKrivanek
Copy link
Member Author

Template package failure may be one of the following. Just wondering if different codes make sense here. E.g. in case download problem, it might be retried; however if package is not found or already installed retry won't help.

Makes sense - I distinguished NotFound case.
Other cases might bee too low level of details for exit code (details are in log if needed).

It would be great if we could distinguish retriable from nonretriable errors by exit codes - unfortunately the DownloadError is still too generic of an error (it includes cases of download file already exist, any generic exception during download, etc.). The nuget API we use (CopyNupkgToStreamAsync), doesn't seem to surface the information - so we'd need a more thorough rework that's beyond scope of this change.

```


## <a name="102"></a>102 - Missing required option(s) and/or argument(s) for the command
Copy link
Member

Choose a reason for hiding this comment

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

i'm still not convinced here. For example below, the command misses required argument, and exit code is 127; as it is managed by parser.

PS > dotnet new install
Required argument missing for command: install



Description:
Installs a template package.



Usage:
dotnet new install [<package>...] [options]



Arguments:
<package> NuGet package ID or path to folder or NuGet package to install.
To install the NuGet package of certain version, use <package ID>::<version>.



Options:
--interactive Allows the command to stop and wait for user input or action (for
example to complete authentication).
--add-source, --nuget-source <nuget-source> Specifies a NuGet source to use during install.
--force Allows installing template packages from the specified sources even if
they would override a template package from another source.
-?, -h, --help Show command line help.





PS > $LASTEXITCODE
127

Search command validation should be moved as the validator. I can work on it next month in scope of parser migration.

@JanKrivanek JanKrivanek requested a review from vlada-shubina June 1, 2022 16:44
Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

Looks good.

Still not too convinced about keeping 102 for now, but i believe we can adjust later on if needed.

@JanKrivanek JanKrivanek merged commit d635904 into dotnet:main Jun 7, 2022
@JanKrivanek JanKrivanek deleted the exit-codes branch June 7, 2022 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

One of the New3CommandStatus values is out of range (silently truncated) [CLI] review exit codes sent via New3CommandResult enum
2 participants