GitHub rate limiting error doesn't cause wingetcreate to fail

[mthalman]

Brief description of your issue

I'm testing some automation using the wingetcreate tool. As part of that, it's invoking the submit command to create a PR. In some cases, I've run into GitHub rate limiting errors. The problem is that when wingetcreate encounters one of these errors, it returns 0 as the exit code for the process. It should be returning a non-zero value so that automation logic knows that an error occurs.

The tool correctly outputs the error message. Here's example output from my scenario:

Manifest validation succeeded: True

Submitting pull request for manifest...

ERROR: You have exceeded a secondary rate limit and have been temporarily blocked from content creation. Please retry your request again later.

But because the process returned 0 for the exit code, my automation logic assumed that everything succeeded and continued processing. Not what I want.

I dug into the code and believe I've identified the cause. The error handling in this code block seems incorrect:

winget-create/src/WingetCreateCLI/Commands/BaseCommand.cs

Lines 491 to 515 in d4606e5

	catch (Exception e)
	{
	TelemetryManager.Log.WriteEvent(new PullRequestEvent
	{
	IsSuccessful = false,
	ErrorMessage = e.Message,
	ExceptionType = e.GetType().ToString(),
	StackTrace = e.StackTrace,
	});
	if (e is Octokit.ForbiddenException)
	{
	Logger.ErrorLocalized(nameof(Resources.Error_Prefix), e.Message);
	return true;
	}
	else if (e is NonFastForwardException nonFastForwardException)
	{
	Logger.ErrorLocalized(nameof(Resources.FastForwardUpdateFailed_Message), nonFastForwardException.CommitsAheadBy);
	return true;
	}
	else
	{
	return false;
	}
	}

It's returning true in exception cases. The GitHubSubmitManifests method documents that true indicates the operation was successful. But that shouldn't be the case for any exception. One other thing I noticed with this implementation is that it only outputs an error message to the Logger for certain exception types. I believe it should always output regardless of the exception type so the user can know what happened.

Steps to reproduce

Invoke the submit command in a way that causes GitHub API to return an error that gets represented as a ForbiddenException or NonFastForwardException. In this case, it was due to rate limiting.

Expected behavior

A non-zero value should be set as the exit code of the wingetcreate process.

Actual behavior

The exit code of wingetcreate is 0.

Environment

Windows Package Manager v1.2.10271
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.22000.556
Package: Microsoft.DesktopAppInstaller v1.17.10271.0

Windows Package Manager Manifest Creator v1.0.3.0