Keep JSON failures and help visible

Author: PrzemyslawKlys

src/AppInstallerCLICore/Command.cpp

@@ -1072,7 +1072,10 @@ namespace AppInstaller::CLI
     {
         try
         {
-            Workflow::SetJsonOutputChannel(context);
+            if (!context.Args.Contains(Execution::Args::Type::Help))
+            {
+                Workflow::SetJsonOutputChannel(context);
+            }
 
             if (!Settings::User().GetWarnings().empty() &&
                 !WI_IsFlagSet(command->GetOutputFlags(), CommandOutputFlags::IgnoreSettingsWarnings))

src/AppInstallerCLICore/Workflows/WorkflowBase.cpp

@@ -508,6 +508,19 @@ namespace AppInstaller::CLI::Workflow
             WriteJsonOutput(context, result);
         }
 
+        Resource::LocString GetUnexpectedErrorMessage(std::string_view message)
+        {
+            std::string result = Resource::LocString{ Resource::String::UnexpectedErrorExecutingCommand }.get();
+
+            if (!message.empty())
+            {
+                result += ' ';
+                result += message;
+            }
+
+            return Resource::LocString{ Utility::LocIndString{ std::move(result) } };
+        }
+
         void OutputInstalledPackagesJson(
             Execution::Context& context,
             std::vector<InstalledPackagesTableLine>& lines,
@@ -790,9 +803,17 @@ namespace AppInstaller::CLI::Workflow
             Logging::Telemetry().LogException(Logging::FailureTypeEnum::ResultException, re.what());
             if (context)
             {
-                context->Reporter.Error() <<
-                    Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
-                    GetUserPresentableMessage(re) << std::endl;
+                auto message = GetUserPresentableMessage(re);
+                if (IsJsonOutputFormat(context->Args))
+                {
+                    OutputInstalledPackagesJsonError(*context, re.GetErrorCode(), GetUnexpectedErrorMessage(message));
+                }
+                else
+                {
+                    context->Reporter.Error() <<
+                        Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
+                        message << std::endl;
+                }
             }
             return re.GetErrorCode();
         }
@@ -802,9 +823,16 @@ namespace AppInstaller::CLI::Workflow
             Logging::Telemetry().LogException(Logging::FailureTypeEnum::WinrtHResultError, message);
             if (context)
             {
-                context->Reporter.Error() <<
-                    Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
-                    message << std::endl;
+                if (IsJsonOutputFormat(context->Args))
+                {
+                    OutputInstalledPackagesJsonError(*context, hre.code(), GetUnexpectedErrorMessage(message));
+                }
+                else
+                {
+                    context->Reporter.Error() <<
+                        Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
+                        message << std::endl;
+                }
             }
             return hre.code();
         }
@@ -832,9 +860,17 @@ namespace AppInstaller::CLI::Workflow
             Logging::Telemetry().LogException(Logging::FailureTypeEnum::StdException, e.what());
             if (context)
             {
-                context->Reporter.Error() <<
-                    Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
-                    GetUserPresentableMessage(e) << std::endl;
+                auto message = GetUserPresentableMessage(e);
+                if (IsJsonOutputFormat(context->Args))
+                {
+                    OutputInstalledPackagesJsonError(*context, APPINSTALLER_CLI_ERROR_COMMAND_FAILED, GetUnexpectedErrorMessage(message));
+                }
+                else
+                {
+                    context->Reporter.Error() <<
+                        Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
+                        message << std::endl;
+                }
             }
             return APPINSTALLER_CLI_ERROR_COMMAND_FAILED;
         }
@@ -844,8 +880,15 @@ namespace AppInstaller::CLI::Workflow
             Logging::Telemetry().LogException(Logging::FailureTypeEnum::Unknown, {});
             if (context)
             {
-                context->Reporter.Error() <<
-                    Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl;
+                if (IsJsonOutputFormat(context->Args))
+                {
+                    OutputInstalledPackagesJsonError(*context, APPINSTALLER_CLI_ERROR_COMMAND_FAILED, GetUnexpectedErrorMessage("???"sv));
+                }
+                else
+                {
+                    context->Reporter.Error() <<
+                        Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl;
+                }
             }
             return APPINSTALLER_CLI_ERROR_COMMAND_FAILED;
         }

src/AppInstallerCLITests/UpdateFlow.cpp

@@ -377,6 +377,23 @@ TEST_CASE("ListFlow_JsonOutputWithSettingsWarnings", "[ListFlow][workflow]")
     REQUIRE(json["packages"].size() == 1);
 }
 
+TEST_CASE("ListFlow_JsonOutputHelpUsesTextOutput", "[ListFlow][workflow]")
+{
+    std::ostringstream listOutput;
+    TestContext context{ listOutput, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    context.Args.AddArg(Execution::Args::Type::Help);
+    context.Args.AddArg(Execution::Args::Type::OutputFormat, "json"sv);
+
+    ListCommand list({});
+    context.SetExecutingCommand(&list);
+    ExecuteWithoutLoggingSuccess(context, &list);
+    INFO(listOutput.str());
+
+    REQUIRE(listOutput.str().find(Resource::String::Usage("winget"_liv, "list"_liv).get()) != std::string::npos);
+    REQUIRE_FALSE(context.IsTerminated());
+}
+
 TEST_CASE("ListFlow_JsonOutputWinGetPolicyDisabled", "[ListFlow][workflow]")
 {
     GroupPolicyTestOverride policies;
@@ -402,6 +419,32 @@ TEST_CASE("ListFlow_JsonOutputWinGetPolicyDisabled", "[ListFlow][workflow]")
     REQUIRE(context.GetTerminationHR() == APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY);
 }
 
+TEST_CASE("ListFlow_JsonOutputGenericExecutionFailure", "[ListFlow][workflow]")
+{
+    std::ostringstream listOutput;
+    TestContext context{ listOutput, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    context.Args.AddArg(Execution::Args::Type::OutputFormat, "json"sv);
+    context.Override({ "OpenSource", [](TestContext&)
+    {
+        THROW_HR(APPINSTALLER_CLI_ERROR_SOURCE_OPEN_FAILED);
+    } });
+
+    ListCommand list({});
+    context.SetExecutingCommand(&list);
+    ExecuteWithoutLoggingSuccess(context, &list);
+    INFO(listOutput.str());
+
+    Json::Value json = ConvertToJson(listOutput.str());
+    REQUIRE(json["packages"].isArray());
+    REQUIRE(json["packages"].empty());
+    REQUIRE(json["sourceFailures"].isArray());
+    REQUIRE(json["sourceFailures"].empty());
+    REQUIRE(json["error"]["code"].asString() == "0x8a150045");
+    REQUIRE(json["error"]["message"].asString().empty() == false);
+    REQUIRE(context.GetTerminationHR() == APPINSTALLER_CLI_ERROR_SOURCE_OPEN_FAILED);
+}
+
 TEST_CASE("ListFlow_JsonOutputBadSource", "[ListFlow][workflow]")
 {
     SetSetting(Stream::UserSources, R"(