From 265972ac24930cfaebb05d1a85344d5e2437ade7 Mon Sep 17 00:00:00 2001 From: Benjamin Nussbaum Date: Tue, 30 Dec 2025 14:09:31 +0100 Subject: [PATCH] fix(#130): server error on faulty recipes --- app/Services/ImageGenerationService.php | 25 ++- .../views/default-screens/error.blade.php | 23 +++ routes/api.php | 24 ++- tests/Feature/Api/DeviceEndpointsTest.php | 161 ++++++++++++++++++ 4 files changed, 221 insertions(+), 12 deletions(-) create mode 100644 resources/views/default-screens/error.blade.php diff --git a/app/Services/ImageGenerationService.php b/app/Services/ImageGenerationService.php index cdfc9d2..fcd5f12 100644 --- a/app/Services/ImageGenerationService.php +++ b/app/Services/ImageGenerationService.php @@ -311,7 +311,7 @@ class ImageGenerationService public static function getDeviceSpecificDefaultImage(Device $device, string $imageType): ?string { // Validate image type - if (! in_array($imageType, ['setup-logo', 'sleep'])) { + if (! in_array($imageType, ['setup-logo', 'sleep', 'error'])) { return null; } @@ -345,10 +345,10 @@ class ImageGenerationService /** * Generate a default screen image from Blade template */ - public static function generateDefaultScreenImage(Device $device, string $imageType): string + public static function generateDefaultScreenImage(Device $device, string $imageType, ?string $pluginName = null): string { // Validate image type - if (! in_array($imageType, ['setup-logo', 'sleep'])) { + if (! in_array($imageType, ['setup-logo', 'sleep', 'error'])) { throw new InvalidArgumentException("Invalid image type: {$imageType}"); } @@ -365,7 +365,7 @@ class ImageGenerationService $outputPath = Storage::disk('public')->path('/images/generated/'.$uuid.'.'.$fileExtension); // Generate HTML from Blade template - $html = self::generateDefaultScreenHtml($device, $imageType); + $html = self::generateDefaultScreenHtml($device, $imageType, $pluginName); // Create custom Browsershot instance if using AWS Lambda $browsershotInstance = null; @@ -445,12 +445,13 @@ class ImageGenerationService /** * Generate HTML from Blade template for default screens */ - private static function generateDefaultScreenHtml(Device $device, string $imageType): string + private static function generateDefaultScreenHtml(Device $device, string $imageType, ?string $pluginName = null): string { // Map image type to template name $templateName = match ($imageType) { 'setup-logo' => 'default-screens.setup', 'sleep' => 'default-screens.sleep', + 'error' => 'default-screens.error', default => throw new InvalidArgumentException("Invalid image type: {$imageType}") }; @@ -461,14 +462,22 @@ class ImageGenerationService $scaleLevel = $device->scaleLevel(); $darkMode = $imageType === 'sleep'; // Sleep mode uses dark mode, setup uses light mode - // Render the Blade template - return view($templateName, [ + // Build view data + $viewData = [ 'noBleed' => false, 'darkMode' => $darkMode, 'deviceVariant' => $deviceVariant, 'deviceOrientation' => $deviceOrientation, 'colorDepth' => $colorDepth, 'scaleLevel' => $scaleLevel, - ])->render(); + ]; + + // Add plugin name for error screens + if ($imageType === 'error' && $pluginName !== null) { + $viewData['pluginName'] = $pluginName; + } + + // Render the Blade template + return view($templateName, $viewData)->render(); } } diff --git a/resources/views/default-screens/error.blade.php b/resources/views/default-screens/error.blade.php new file mode 100644 index 0000000..be8063a --- /dev/null +++ b/resources/views/default-screens/error.blade.php @@ -0,0 +1,23 @@ +@props([ + 'noBleed' => false, + 'darkMode' => false, + 'deviceVariant' => 'og', + 'deviceOrientation' => null, + 'colorDepth' => '1bit', + 'scaleLevel' => null, + 'pluginName' => 'Recipe', +]) + + + + + + Error on {{ $pluginName }} + Unable to render content. Please check server logs. + + + + + diff --git a/routes/api.php b/routes/api.php index d1dbcac..b1d08b4 100644 --- a/routes/api.php +++ b/routes/api.php @@ -95,9 +95,16 @@ Route::get('/display', function (Request $request) { // Check and update stale data if needed if ($plugin->isDataStale() || $plugin->current_image === null) { $plugin->updateDataPayload(); - $markup = $plugin->render(device: $device); + try { + $markup = $plugin->render(device: $device); - GenerateScreenJob::dispatchSync($device->id, $plugin->id, $markup); + GenerateScreenJob::dispatchSync($device->id, $plugin->id, $markup); + } catch (Exception $e) { + Log::error("Failed to render plugin {$plugin->id} ({$plugin->name}): ".$e->getMessage()); + // Generate error display + $errorImageUuid = ImageGenerationService::generateDefaultScreenImage($device, 'error', $plugin->name); + $device->update(['current_screen_image' => $errorImageUuid]); + } } $plugin->refresh(); @@ -120,8 +127,17 @@ Route::get('/display', function (Request $request) { } } - $markup = $playlistItem->render(device: $device); - GenerateScreenJob::dispatchSync($device->id, null, $markup); + try { + $markup = $playlistItem->render(device: $device); + GenerateScreenJob::dispatchSync($device->id, null, $markup); + } catch (Exception $e) { + Log::error("Failed to render mashup playlist item {$playlistItem->id}: ".$e->getMessage()); + // For mashups, show error for the first plugin or a generic error + $firstPlugin = $plugins->first(); + $pluginName = $firstPlugin ? $firstPlugin->name : 'Recipe'; + $errorImageUuid = ImageGenerationService::generateDefaultScreenImage($device, 'error', $pluginName); + $device->update(['current_screen_image' => $errorImageUuid]); + } $device->refresh(); diff --git a/tests/Feature/Api/DeviceEndpointsTest.php b/tests/Feature/Api/DeviceEndpointsTest.php index aff6758..2925a5e 100644 --- a/tests/Feature/Api/DeviceEndpointsTest.php +++ b/tests/Feature/Api/DeviceEndpointsTest.php @@ -7,6 +7,7 @@ use App\Models\Playlist; use App\Models\PlaylistItem; use App\Models\Plugin; use App\Models\User; +use App\Services\ImageGenerationService; use Bnussbau\TrmnlPipeline\TrmnlPipeline; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; @@ -1023,3 +1024,163 @@ test('screens endpoint matches MAC address case-insensitively', function (): voi $response->assertOk(); Queue::assertPushed(GenerateScreenJob::class); }); + +test('display endpoint handles plugin rendering errors gracefully', function (): void { + TrmnlPipeline::fake(); + + $device = Device::factory()->create([ + 'mac_address' => '00:11:22:33:44:55', + 'api_key' => 'test-api-key', + 'proxy_cloud' => false, + ]); + + // Create a plugin with Blade markup that will cause an exception when accessing data[0] + // when data is not an array or doesn't have index 0 + $plugin = Plugin::factory()->create([ + 'name' => 'Broken Recipe', + 'data_strategy' => 'polling', + 'polling_url' => null, + 'data_stale_minutes' => 1, + 'markup_language' => 'blade', // Use Blade which will throw exception on invalid array access + 'render_markup' => '
{{ $data[0]["invalid"] }}
', // This will fail if data[0] doesn't exist + 'data_payload' => ['error' => 'Failed to fetch data'], // Not a list, so data[0] will fail + 'data_payload_updated_at' => now()->subMinutes(2), // Make it stale + 'current_image' => null, + ]); + + $playlist = Playlist::factory()->create([ + 'device_id' => $device->id, + 'name' => 'test_playlist', + 'is_active' => true, + 'weekdays' => null, + 'active_from' => null, + 'active_until' => null, + ]); + + PlaylistItem::factory()->create([ + 'playlist_id' => $playlist->id, + 'plugin_id' => $plugin->id, + 'order' => 1, + 'is_active' => true, + 'last_displayed_at' => null, + ]); + + $response = $this->withHeaders([ + 'id' => $device->mac_address, + 'access-token' => $device->api_key, + 'rssi' => -70, + 'battery_voltage' => 3.8, + 'fw-version' => '1.0.0', + ])->get('/api/display'); + + $response->assertOk(); + + // Verify error screen was generated and set on device + $device->refresh(); + expect($device->current_screen_image)->not->toBeNull(); + + // Verify the error image exists + $errorImagePath = Storage::disk('public')->path("images/generated/{$device->current_screen_image}.png"); + // The TrmnlPipeline is faked, so we just verify the UUID was set + expect($device->current_screen_image)->toBeString(); +}); + +test('display endpoint handles mashup rendering errors gracefully', function (): void { + TrmnlPipeline::fake(); + + $device = Device::factory()->create([ + 'mac_address' => '00:11:22:33:44:55', + 'api_key' => 'test-api-key', + 'proxy_cloud' => false, + ]); + + // Create plugins for mashup, one with invalid markup + $plugin1 = Plugin::factory()->create([ + 'name' => 'Working Plugin', + 'data_strategy' => 'polling', + 'polling_url' => null, + 'data_stale_minutes' => 1, + 'render_markup_view' => 'trmnl', + 'data_payload_updated_at' => now()->subMinutes(2), + 'current_image' => null, + ]); + + $plugin2 = Plugin::factory()->create([ + 'name' => 'Broken Plugin', + 'data_strategy' => 'polling', + 'polling_url' => null, + 'data_stale_minutes' => 1, + 'markup_language' => 'blade', // Use Blade which will throw exception on invalid array access + 'render_markup' => '
{{ $data[0]["invalid"] }}
', // This will fail + 'data_payload' => ['error' => 'Failed to fetch data'], + 'data_payload_updated_at' => now()->subMinutes(2), + 'current_image' => null, + ]); + + $playlist = Playlist::factory()->create([ + 'device_id' => $device->id, + 'name' => 'test_playlist', + 'is_active' => true, + 'weekdays' => null, + 'active_from' => null, + 'active_until' => null, + ]); + + // Create mashup playlist item + $playlistItem = PlaylistItem::createMashup( + $playlist, + '1Lx1R', + [$plugin1->id, $plugin2->id], + 'Test Mashup', + 1 + ); + + $response = $this->withHeaders([ + 'id' => $device->mac_address, + 'access-token' => $device->api_key, + 'rssi' => -70, + 'battery_voltage' => 3.8, + 'fw-version' => '1.0.0', + ])->get('/api/display'); + + $response->assertOk(); + + // Verify error screen was generated and set on device + $device->refresh(); + expect($device->current_screen_image)->not->toBeNull(); + + // Verify the error image UUID was set + expect($device->current_screen_image)->toBeString(); +}); + +test('generateDefaultScreenImage creates error screen with plugin name', function (): void { + TrmnlPipeline::fake(); + Storage::fake('public'); + Storage::disk('public')->makeDirectory('/images/generated'); + + $device = Device::factory()->create(); + + $errorUuid = ImageGenerationService::generateDefaultScreenImage($device, 'error', 'Test Recipe Name'); + + expect($errorUuid)->not->toBeEmpty(); + + // Verify the error image path would be created + $errorPath = "images/generated/{$errorUuid}.png"; + // Since TrmnlPipeline is faked, we just verify the UUID was generated + expect($errorUuid)->toBeString(); +}); + +test('generateDefaultScreenImage throws exception for invalid error image type', function (): void { + $device = Device::factory()->create(); + + expect(fn (): string => ImageGenerationService::generateDefaultScreenImage($device, 'invalid-error-type')) + ->toThrow(InvalidArgumentException::class); +}); + +test('getDeviceSpecificDefaultImage returns null for error type when no device-specific image exists', function (): void { + $device = new Device(); + $device->deviceModel = null; + + $result = ImageGenerationService::getDeviceSpecificDefaultImage($device, 'error'); + expect($result)->toBeNull(); +});