From 29280cd950cc3ddecc4262cadab2a7262b6fd2cc Mon Sep 17 00:00:00 2001
From: Your Name <you@example.com>
Date: Wed, 2 Sep 2026 19:15:41 +0930
Subject: [PATCH] Fix silent failure in avatar upload endpoints

AvatarController@store and BaseApiController@avatarUpdate wrapped the
upload flow in an empty catch(\Exception) block and returned a success
response even when the upload or save failed.

Log the exception and return a real error response (500 JSON for the
API endpoint, a redirect with validation errors for the web endpoint).

Adds regression tests covering the failure path, the success path, and
non-image rejection.
---
 .../Controllers/Api/BaseApiController.php     | 10 +++
 app/Http/Controllers/AvatarController.php     |  9 +++
 tests/Feature/Api/AvatarUpdateTest.php        | 73 +++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 tests/Feature/Api/AvatarUpdateTest.php

diff --git a/app/Http/Controllers/Api/BaseApiController.php b/app/Http/Controllers/Api/BaseApiController.php
index 38b6e9006..9358b515e 100644
--- a/app/Http/Controllers/Api/BaseApiController.php
+++ b/app/Http/Controllers/Api/BaseApiController.php
@@ -18,6 +18,7 @@ use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
 use League\Fractal;
 use League\Fractal\Serializer\ArraySerializer;
 
@@ -109,6 +110,15 @@ class BaseApiController extends Controller
             Cache::forget("avatar:{$profile->id}");
             AvatarOptimize::dispatch($user->profile, $currentAvatar);
         } catch (\Exception $e) {
+            Log::error('BaseApiController@avatarUpdate failed: '.$e->getMessage(), [
+                'user_id' => $request->user()?->id,
+                'exception' => $e,
+            ]);
+
+            return response()->json([
+                'code' => 500,
+                'msg' => 'There was an error updating your avatar. Please try again.',
+            ], 500);
         }
 
         return response()->json([
diff --git a/app/Http/Controllers/AvatarController.php b/app/Http/Controllers/AvatarController.php
index d37ef4dab..c3dbc502e 100644
--- a/app/Http/Controllers/AvatarController.php
+++ b/app/Http/Controllers/AvatarController.php
@@ -8,6 +8,7 @@ use Illuminate\Http\JsonResponse;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Str;
 
 class AvatarController extends Controller
@@ -44,6 +45,14 @@ class AvatarController extends Controller
             Cache::forget('user:account:id:'.$user->id);
             AvatarOptimize::dispatch($user->profile, $currentAvatar);
         } catch (\Exception $e) {
+            Log::error('AvatarController@store failed: '.$e->getMessage(), [
+                'user_id' => $request->user()?->id,
+                'exception' => $e,
+            ]);
+
+            return redirect()->back()->withErrors([
+                'avatar' => 'There was an error updating your avatar. Please try again.',
+            ]);
         }
 
         return redirect()->back()->with('status', 'Avatar updated successfully. It may take a few minutes to update across the site.');
diff --git a/tests/Feature/Api/AvatarUpdateTest.php b/tests/Feature/Api/AvatarUpdateTest.php
new file mode 100644
index 000000000..3e654c640
--- /dev/null
+++ b/tests/Feature/Api/AvatarUpdateTest.php
@@ -0,0 +1,73 @@
+<?php
+
+use App\Models\Avatar;
+use App\Models\User;
+use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Storage;
+use Laravel\Passport\Passport;
+
+uses(LazilyRefreshDatabase::class);
+
+/*
+|--------------------------------------------------------------------------
+| Avatar Update API Tests
+|--------------------------------------------------------------------------
+|
+| Regression coverage for the previously silent failure in
+| BaseApiController@avatarUpdate, where any exception during upload was
+| swallowed and the endpoint still returned a 200 "success" response.
+|
+*/
+
+describe('POST /api/v1/avatar/update', function () {
+    it('returns an error instead of a false success when processing fails', function () {
+        $user = User::factory()->create();
+        $user->refresh();
+
+        // Ensure no Avatar row exists so the internal firstOrFail() throws.
+        Avatar::whereProfileId($user->profile_id)->delete();
+
+        Storage::fake('local');
+        Passport::actingAs($user, ['write']);
+
+        $this->postJson('/api/v1/avatar/update', [
+            'upload' => UploadedFile::fake()->image('avatar.jpg', 200, 200),
+        ])
+            ->assertStatus(500)
+            ->assertJsonFragment(['code' => 500])
+            ->assertJsonMissing(['msg' => 'Avatar successfully updated']);
+    });
+
+    it('updates the avatar with a valid upload', function () {
+        $user = User::factory()->create();
+        $user->refresh();
+
+        Avatar::updateOrCreate(
+            ['profile_id' => $user->profile_id],
+            ['media_path' => 'public/avatars/default.jpg', 'change_count' => 0]
+        );
+
+        Storage::fake('local');
+        Passport::actingAs($user, ['write']);
+
+        $this->postJson('/api/v1/avatar/update', [
+            'upload' => UploadedFile::fake()->image('avatar.jpg', 200, 200),
+        ])
+            ->assertOk()
+            ->assertJsonFragment(['msg' => 'Avatar successfully updated']);
+    });
+
+    it('rejects a non-image upload with a validation error', function () {
+        $user = User::factory()->create();
+        $user->refresh();
+
+        Storage::fake('local');
+        Passport::actingAs($user, ['write']);
+
+        $this->postJson('/api/v1/avatar/update', [
+            'upload' => UploadedFile::fake()->create('malware.pdf', 100, 'application/pdf'),
+        ])
+            ->assertStatus(422);
+    });
+});