Presumably this is because the REST endpoint was not integrated with MWOAuthSessionProvider::makeException.

Thank you @Tgr . I confirmed this locally by putting this little snippet in rest.php, and I got a much happier message:

try {
	$main = new ApiMain();
	Hooks::run( 'ApiBeforeMain', [ &$main ] );
	EntryPoint::main();
} catch ( Throwable $e ) {
	wfHttpError( 500, 'Exception', $e->getMessage() );
}

Clearly, this code is not suitable for production for several reasons. Among other things, creating an empty ApiMain just to run the hook is silly.

This is my first experience with ApiBeforeMain, so there may be context I'm missing. It would be straightforward to create a new hook for use by the REST API and give it parallel treatment in MWOAuthSessionProvider::makeException. But that feels a little messy. Is there something cleaner you'd suggest?

The fundamental issue here is that the session is initialized in Setup.php, which is outside of the main try..catch block of the API and so the exception would not go through the usual error logging / output formatting logic. So one option is to make that logic not rely on the try block and register it early as the PHP error handler; but it's probably easy to get into service dependency loops that way.

Other than that, I'm not sure what could be changed. If we rely on the exception being thrown inside the main try..catch block, it has to be caught soon enough that the catching code knows how to intelligently handle it (by setting up an empty anonymous session). The exception could be left to bubble up all the way to the caller of SessionManager::getSessionForRequest and handled there, but that doesn't really seem cleaner.

So probably the only thing I'd change were to move makeException to a helper method available to any session provider. CentralAuthTokenSessionProvider also uses it, and it would be needed by any other provider where failure to authenticate means no access instead of anonymous user access.

Change 597370 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/core@master] Add new hook RestApiBeforeExecuteHandler

https://gerrit.wikimedia.org/r/597370

Change 597372 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/extensions/OAuth@master] Call new hook RestApiBeforeExecuteHandler on exception

https://gerrit.wikimedia.org/r/597372

Uploaded patch to integrate a new hook - RestApiBeforeExecuteHandler - with the Core REST API infrastructure. I introduced a new hook because ApiBeforeMain requires an ApiMain instance, and ApiMain isn't used by the Core REST API infrastructure.

Uploaded a separate patch to use the new hook in the OAuth extension.

I did not introduce a helper function for makeException based on the Rule of Three, but that's a good suggestion to keep in mind if this comes up again.

Still to do: update CentralAuth to use the new hook. Thought I'd get feedback on the first two patches before creating that one.

I have issues with this approach. Copied from gerrit:

Alternatively to a hook you could add a new field in the SessionInfo, store error in there and then check for it in MWBasicAuthorizer. Approach with the hook sucks cause the session initialization code indirectly needs to know about what is using it afterwards. The existence of this patch is a demonstration of the problem with the hook approach. I won't veto merging this, but I think there's opportunity for improvement here. Basically you abuse the hook system for passing around the information - the registration of the hook handler serves as a way to pass an error flag.

This needs more attention based on comments from reviewers. Moving back to "Ready" until I start working on this again, to more accurately reflect its current status.

It seems like the problem here is code that runs prior to endpoint specific code needs a generic way to inform the output layer that what error has occured. Today what we are doing is registering hook handlers for each specific output layer, and throwing different output specific exceptions.

An idea could be a collector we register to the service container. Anything running too-early in the system could add it's errors there. Each endpoint would query it during their own initialization. After querying it the collector would disable itself (throw exceptions) on future attempts to register errors to prevent registering errors too-late in the request. Could call it SetupErrorCollector or some such.

In T252591#9507023, @EBernhardson wrote:

An idea could be a collector we register to the service container. Anything running too-early in the system could add it's errors there. Each endpoint would query it during their own initialization. After querying it the collector would disable itself (throw exceptions) on future attempts to register errors to prevent registering errors too-late in the request. Could call it SetupErrorCollector or some such.

+1, I was thinking along the same lines when this came up previously (but then forgot about it).