Exceptions should be the recommended way of causing HTTP error codes

[jez9999]

Warning: contraversial opinion incoming...

Summary

The official Microsoft guidance on Exception usage should be loosened to allow and even recommend the use of exceptions in certain circumstances, like ASP.NET WebApi.

Motivation and goals

I've been reading up on recommended Exception usage and thinking about it over the years. The "best practices" documentation is all somewhat subjective and contradictory IMHO: here it says to handle common conditions without throwing exceptions, but just down the page it says to throw exceptions instead of returning an error code. If your WebApi controller is calling a class that tries to do something based on input the API has received, what should happen when that input is invalid and execution cannot continue? Should the internal class throw an exception and the app use a custom exception handler to format and return an appropriate HTTP response, or should the internal class use a try/parse pattern to return a tuple containing "wasSuccess" along with the "result", in the case of success? This rather contradictory recommendation is repeated here: "DO NOT return error codes.", "DO NOT use exceptions for the normal flow of control, if possible." (but you need to basically return error codes to indicate to the web API controller what kind of HTTP response to generate).

Examples

Using an exception to cause a failure response (an exception handler is able to catch LightException, a class which is able to contain all the necessary information needed by the API to format an appropriate HTTP error response; nature of error, error msg, etc.):

WebAPI controller

// POST: api/games
[HttpPost("")]
public async Task<ActionResult<CreatedGameInfo>> CreateGame(CreateGameConfig newGame) {
	// Create game
	var gameData = await _gameCreator.CreateGame(newGame.Name, newGame.NbrPlayers, newGame.HexAllocation);

	// Serialize it to DB
	// [...]
	if (gameData.Field.ContainsHexAt(1, 2)) {
		// [...]
	}
	// [...]

	return new CreatedGameInfo(
		gameData.Id,
		gameData.Slug,
		"comms1"
	);
}

GameCreator class

public async Task<GameData> CreateGame(string? gameName, string? nbrPlayersStr, string? hexAllocationStr) {
	if (gameName == null) {
		throw new LightException("When creating a game, a game name must be provided.", ErrorType.UserInput);
	}
	else if (nbrPlayersStr == null) {
		throw new LightException("When creating a game, number of players must be provided.", ErrorType.UserInput);
	}
	else if (hexAllocationStr == null) {
		throw new LightException("When creating a game, hex allocation must be provided.", ErrorType.UserInput);
	}

	if (!int.TryParse(nbrPlayersStr, out int nbrPlayers)) {
		throw new LightException($"Couldn't parse number of players value: {nbrPlayersStr}", ErrorType.InputParsing);
	}
	if (!hexAllocationStr.TryToEnumFromName(out HexAllocation hexAllocation)) {
		throw new LightException($"Couldn't parse hex allocation value: {hexAllocationStr}", ErrorType.InputParsing);
	}

	return CreateRandomGame(RulesetVersion.V1, gameName, nbrPlayers, 30, 20, hexAllocation);
	// ^ May throw LightException with ErrorType.Generation, "Couldn't generate valid game map from random data"
}

Using the try/parse pattern instead, it looks more like this (API controller takes responsibility for handling error... and how different is this, conceptually, from CreateGame "returning an error code"? It could be missed by the controller):

WebAPI controller

// POST: api/games
[HttpPost("")]
public async Task<ActionResult<CreatedGameInfo>> CreateGame(CreateGameConfig newGame) {
	// Create game
	var gameData = await _gameCreator.TryCreateGame(newGame.Name, newGame.NbrPlayers, newGame.HexAllocation);
	if (gameData.ErrorMsg != null) {
		// Bad Request assumed, would need even more data returned into gameData to know more
		return BadRequest(gameData.ErrorMsg);
	}

	// Serialize it to DB
	// [...]
	// Null-forgiving operator needed because Result needed to be nullable in case of error
	if (gameData.Result!.Field.ContainsHexAt(1, 2)) {
		// [...]
	}
	// [...]

	return new CreatedGameInfo(
		gameData.Result!.Id,
		gameData.Result!.Slug,
		"comms1"
	);
}

GameCreator class

public async Task<(string? ErrorMsg, GameData? Result)> TryCreateGame(string? gameName, string? nbrPlayersStr, string? hexAllocationStr) {...}

Risks / unknowns

It seems to me that the exception mechanism provides a very nice way to interrupt code execution in the case of a failure because of either invalid input, or some kind of internal problem, package up the data needed to construct a correct HTTP error response, and allow the calling code to assume success if the method called didn't throw an exception, removing the need for it to constantly worry about return values indicating an error.

As far as I can tell, this isn't really objected to in principle: the reason it is (somewhat) advised against is for performance reasons. Although I would note that the documentation talks about "throw rates above 100 per second" being likely to cause performance issues (how many web APIs are really going to get to that point even when using exceptions for normal error flow?), isn't this an argument for some kind of more lightweight exception mechanism to allow the throwing of exceptions with better performance upon a known error condition when a call stack isn't needed and so some of the inefficiency of exceptions can be eliminated? LightException could just be an object that contains only the information given to it by the code throwing the exception.

I'm also not really convinced that, for the majority of web APIs, the current exception performance is really going to be a problem. A throw rate above 100 per second seems like something that would only happen during a DOS attack because generally an API is going to be fed valid input that has been validated in client-side code before being submitted.

Isn't it a shame to use a more clunky error handling mechanism for performance reasons when, semantically, exceptions allow for less verbose code and eliminate the risk of API controller methods missing the error condition?

[ghost]

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!