From efa9d38d73d5faf18b9f47eef6603e2e1dc45527 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <sasha.skvortsov109@gmail.com>
Date: Mon, 23 Aug 2021 12:44:22 -0400
Subject: [PATCH] Throw exceptions in API Client responses

Currently, the API client middleware includes an error handler instance in its middleware stack, so any exceptions thrown have to be manually checked for in API client callers. This is generally forgotten or omited, and leads to issues when call sites try to read data from the response but fail with a confusing error.

In this PR, we no longer handle those errors, so they will be propogated in their original form to the original request's error handler. This is more appropriate behavior, and will make debugging errors significantly easier. This is not a breaking change, since broken requests would have failed anyway due to other, more confusing errors. Additionally, all error checking code that I've found just throws a new error if an API client request fails, so that case won't be broken either.
---
 src/Api/ApiServiceProvider.php           |  1 +
 src/Forum/Content/Discussion.php         |  5 -----
 src/Forum/Content/User.php               |  5 -----
 src/Forum/Controller/LogInController.php | 16 +++++++---------
 4 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/src/Api/ApiServiceProvider.php b/src/Api/ApiServiceProvider.php
index 7d18e806b..84791f4bb 100644
--- a/src/Api/ApiServiceProvider.php
+++ b/src/Api/ApiServiceProvider.php
@@ -106,6 +106,7 @@ class ApiServiceProvider extends AbstractServiceProvider
         $this->container->singleton('flarum.api_client.exclude_middleware', function () {
             return [
                 HttpMiddleware\InjectActorReference::class,
+                'flarum.api.error_handler',
                 HttpMiddleware\ParseJsonBody::class,
                 Middleware\FakeHttpMethods::class,
                 HttpMiddleware\StartSession::class,
diff --git a/src/Forum/Content/Discussion.php b/src/Forum/Content/Discussion.php
index 936e0e328..dc4087988 100644
--- a/src/Forum/Content/Discussion.php
+++ b/src/Forum/Content/Discussion.php
@@ -116,11 +116,6 @@ class Discussion
             ->withParentRequest($request)
             ->withQueryParams($params)
             ->get("/discussions/$id");
-        $statusCode = $response->getStatusCode();
-
-        if ($statusCode === 404) {
-            throw new RouteNotFoundException;
-        }
 
         return json_decode($response->getBody());
     }
diff --git a/src/Forum/Content/User.php b/src/Forum/Content/User.php
index 50a67f761..0c72fad36 100644
--- a/src/Forum/Content/User.php
+++ b/src/Forum/Content/User.php
@@ -61,11 +61,6 @@ class User
     protected function getApiDocument(Request $request, string $username)
     {
         $response = $this->api->withParentRequest($request)->withQueryParams(['bySlug' => true])->get("/users/$username");
-        $statusCode = $response->getStatusCode();
-
-        if ($statusCode === 404) {
-            throw new ModelNotFoundException;
-        }
 
         return json_decode($response->getBody());
     }
diff --git a/src/Forum/Controller/LogInController.php b/src/Forum/Controller/LogInController.php
index 30212d3b8..3c98d4b3f 100644
--- a/src/Forum/Controller/LogInController.php
+++ b/src/Forum/Controller/LogInController.php
@@ -74,19 +74,17 @@ class LogInController implements RequestHandlerInterface
 
         $response = $this->apiClient->withParentRequest($request)->withBody($params)->post('/token');
 
-        if ($response->getStatusCode() === 200) {
-            $data = json_decode($response->getBody());
+        $data = json_decode($response->getBody());
 
-            $token = AccessToken::findValid($data->token);
+        $token = AccessToken::findValid($data->token);
 
-            $session = $request->getAttribute('session');
-            $this->authenticator->logIn($session, $token);
+        $session = $request->getAttribute('session');
+        $this->authenticator->logIn($session, $token);
 
-            $this->events->dispatch(new LoggedIn($this->users->findOrFail($data->userId), $token));
+        $this->events->dispatch(new LoggedIn($this->users->findOrFail($data->userId), $token));
 
-            if ($token instanceof RememberAccessToken) {
-                $response = $this->rememberer->remember($response, $token);
-            }
+        if ($token instanceof RememberAccessToken) {
+            $response = $this->rememberer->remember($response, $token);
         }
 
         return $response;