fix(server): http error parsing on endpoints without a default response (#12927)

mobile/openapi/README.md

@@ -448,6 +448,7 @@ Class | Method | HTTP request | Description
  - [TagUpsertDto](doc//TagUpsertDto.md)
  - [TagsResponse](doc//TagsResponse.md)
  - [TagsUpdate](doc//TagsUpdate.md)
+ - [TestEmailResponseDto](doc//TestEmailResponseDto.md)
  - [TimeBucketResponseDto](doc//TimeBucketResponseDto.md)
  - [TimeBucketSize](doc//TimeBucketSize.md)
  - [ToneMapping](doc//ToneMapping.md)

mobile/openapi/lib/api.dart

@@ -260,6 +260,7 @@ part 'model/tag_update_dto.dart';
 part 'model/tag_upsert_dto.dart';
 part 'model/tags_response.dart';
 part 'model/tags_update.dart';
+part 'model/test_email_response_dto.dart';
 part 'model/time_bucket_response_dto.dart';
 part 'model/time_bucket_size.dart';
 part 'model/tone_mapping.dart';

mobile/openapi/lib/api/notifications_api.dart

@@ -48,10 +48,18 @@ class NotificationsApi {
   /// Parameters:
   ///
   /// * [SystemConfigSmtpDto] systemConfigSmtpDto (required):
-  Future<void> sendTestEmail(SystemConfigSmtpDto systemConfigSmtpDto,) async {
+  Future<TestEmailResponseDto?> sendTestEmail(SystemConfigSmtpDto systemConfigSmtpDto,) async {
     final response = await sendTestEmailWithHttpInfo(systemConfigSmtpDto,);
     if (response.statusCode >= HttpStatus.badRequest) {
       throw ApiException(response.statusCode, await _decodeBodyBytes(response));
     }
+    // When a remote server returns no body with a status of 204, we shall not decode it.
+    // At the time of writing this, `dart:convert` will throw an "Unexpected end of input"
+    // FormatException when trying to decode an empty string.
+    if (response.body.isNotEmpty && response.statusCode != HttpStatus.noContent) {
+      return await apiClient.deserializeAsync(await _decodeBodyBytes(response), 'TestEmailResponseDto',) as TestEmailResponseDto;
+    
+    }
+    return null;
   }
 }

mobile/openapi/lib/api_client.dart

@@ -574,6 +574,8 @@ class ApiClient {
           return TagsResponse.fromJson(value);
         case 'TagsUpdate':
           return TagsUpdate.fromJson(value);
+        case 'TestEmailResponseDto':
+          return TestEmailResponseDto.fromJson(value);
         case 'TimeBucketResponseDto':
           return TimeBucketResponseDto.fromJson(value);
         case 'TimeBucketSize':

mobile/openapi/lib/model/test_email_response_dto.dart

@@ -0,0 +1,99 @@
+//
+// AUTO-GENERATED FILE, DO NOT MODIFY!
+//
+// @dart=2.18
+
+// ignore_for_file: unused_element, unused_import
+// ignore_for_file: always_put_required_named_parameters_first
+// ignore_for_file: constant_identifier_names
+// ignore_for_file: lines_longer_than_80_chars
+
+part of openapi.api;
+
+class TestEmailResponseDto {
+  /// Returns a new [TestEmailResponseDto] instance.
+  TestEmailResponseDto({
+    required this.messageId,
+  });
+
+  String messageId;
+
+  @override
+  bool operator ==(Object other) => identical(this, other) || other is TestEmailResponseDto &&
+    other.messageId == messageId;
+
+  @override
+  int get hashCode =>
+    // ignore: unnecessary_parenthesis
+    (messageId.hashCode);
+
+  @override
+  String toString() => 'TestEmailResponseDto[messageId=$messageId]';
+
+  Map<String, dynamic> toJson() {
+    final json = <String, dynamic>{};
+      json[r'messageId'] = this.messageId;
+    return json;
+  }
+
+  /// Returns a new [TestEmailResponseDto] instance and imports its values from
+  /// [value] if it's a [Map], null otherwise.
+  // ignore: prefer_constructors_over_static_methods
+  static TestEmailResponseDto? fromJson(dynamic value) {
+    upgradeDto(value, "TestEmailResponseDto");
+    if (value is Map) {
+      final json = value.cast<String, dynamic>();
+
+      return TestEmailResponseDto(
+        messageId: mapValueOfType<String>(json, r'messageId')!,
+      );
+    }
+    return null;
+  }
+
+  static List<TestEmailResponseDto> listFromJson(dynamic json, {bool growable = false,}) {
+    final result = <TestEmailResponseDto>[];
+    if (json is List && json.isNotEmpty) {
+      for (final row in json) {
+        final value = TestEmailResponseDto.fromJson(row);
+        if (value != null) {
+          result.add(value);
+        }
+      }
+    }
+    return result.toList(growable: growable);
+  }
+
+  static Map<String, TestEmailResponseDto> mapFromJson(dynamic json) {
+    final map = <String, TestEmailResponseDto>{};
+    if (json is Map && json.isNotEmpty) {
+      json = json.cast<String, dynamic>(); // ignore: parameter_assignments
+      for (final entry in json.entries) {
+        final value = TestEmailResponseDto.fromJson(entry.value);
+        if (value != null) {
+          map[entry.key] = value;
+        }
+      }
+    }
+    return map;
+  }
+
+  // maps a json object with a list of TestEmailResponseDto-objects as value to a dart map
+  static Map<String, List<TestEmailResponseDto>> mapListFromJson(dynamic json, {bool growable = false,}) {
+    final map = <String, List<TestEmailResponseDto>>{};
+    if (json is Map && json.isNotEmpty) {
+      // ignore: parameter_assignments
+      json = json.cast<String, dynamic>();
+      for (final entry in json.entries) {
+        map[entry.key] = TestEmailResponseDto.listFromJson(entry.value, growable: growable,);
+      }
+    }
+    return map;
+  }
+
+  /// The list of required keys that must be present in a JSON.
+  static const requiredKeys = <String>{
+    'messageId',
+  };
+}
+

open-api/immich-openapi-specs.json

@@ -3491,6 +3491,13 @@
         },
         "responses": {
           "200": {
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/TestEmailResponseDto"
+                }
+              }
+            },
             "description": ""
           }
         },
@@ -12348,6 +12355,17 @@
         },
         "type": "object"
       },
+      "TestEmailResponseDto": {
+        "properties": {
+          "messageId": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "messageId"
+        ],
+        "type": "object"
+      },
       "TimeBucketResponseDto": {
         "properties": {
           "count": {

open-api/typescript-sdk/src/fetch-client.ts

@@ -656,6 +656,9 @@ export type SystemConfigSmtpDto = {
     replyTo: string;
     transport: SystemConfigSmtpTransportDto;
 };
+export type TestEmailResponseDto = {
+    messageId: string;
+};
 export type OAuthConfigDto = {
     redirectUri: string;
 };
@@ -2220,7 +2223,10 @@ export function addMemoryAssets({ id, bulkIdsDto }: {
 export function sendTestEmail({ systemConfigSmtpDto }: {
     systemConfigSmtpDto: SystemConfigSmtpDto;
 }, opts?: Oazapfts.RequestOpts) {
-    return oazapfts.ok(oazapfts.fetchText("/notifications/test-email", oazapfts.json({
+    return oazapfts.ok(oazapfts.fetchJson<{
+        status: 200;
+        data: TestEmailResponseDto;
+    }>("/notifications/test-email", oazapfts.json({
         ...opts,
         method: "POST",
         body: systemConfigSmtpDto

server/src/controllers/notification.controller.ts

@@ -1,6 +1,7 @@
 import { Body, Controller, HttpCode, HttpStatus, Post } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
 import { AuthDto } from 'src/dtos/auth.dto';
+import { TestEmailResponseDto } from 'src/dtos/notification.dto';
 import { SystemConfigSmtpDto } from 'src/dtos/system-config.dto';
 import { Auth, Authenticated } from 'src/middleware/auth.guard';
 import { NotificationService } from 'src/services/notification.service';
@@ -13,7 +14,7 @@ export class NotificationController {
   @Post('test-email')
   @HttpCode(HttpStatus.OK)
   @Authenticated({ admin: true })
-  sendTestEmail(@Auth() auth: AuthDto, @Body() dto: SystemConfigSmtpDto) {
+  sendTestEmail(@Auth() auth: AuthDto, @Body() dto: SystemConfigSmtpDto): Promise<TestEmailResponseDto> {
     return this.service.sendTestEmail(auth.user.id, dto);
   }
 }

server/src/dtos/notification.dto.ts

@@ -0,0 +1,3 @@
+export class TestEmailResponseDto {
+  messageId!: string;
+}

server/src/services/notification.service.spec.ts

@@ -616,11 +616,6 @@ describe(NotificationService.name, () => {
       await expect(sut.handleSendEmail({ html: '', subject: '', text: '', to: '' })).resolves.toBe(JobStatus.SKIPPED);
     });
 
-    it('should fail if email could not be sent', async () => {
-      systemMock.get.mockResolvedValue({ notifications: { smtp: { enabled: true } } });
-      await expect(sut.handleSendEmail({ html: '', subject: '', text: '', to: '' })).resolves.toBe(JobStatus.FAILED);
-    });
-
     it('should send mail successfully', async () => {
       systemMock.get.mockResolvedValue({ notifications: { smtp: { enabled: true, from: 'test@immich.app' } } });
       notificationMock.sendEmail.mockResolvedValue({ messageId: '', response: '' });

server/src/services/notification.service.ts

@@ -1,4 +1,4 @@
-import { HttpException, HttpStatus, Inject, Injectable } from '@nestjs/common';
+import { BadRequestException, Inject, Injectable } from '@nestjs/common';
 import { DEFAULT_EXTERNAL_DOMAIN } from 'src/constants';
 import { SystemConfigCore } from 'src/cores/system-config.core';
 import { OnEmit } from 'src/decorators';
@@ -140,7 +140,7 @@ export class NotificationService {
     try {
       await this.notificationRepository.verifySmtp(dto.transport);
     } catch (error) {
-      throw new HttpException('Failed to verify SMTP configuration', HttpStatus.BAD_REQUEST, { cause: error });
+      throw new BadRequestException('Failed to verify SMTP configuration', { cause: error });
     }
 
     const { server } = await this.configCore.getConfig({ withCache: false });
@@ -152,7 +152,7 @@ export class NotificationService {
       },
     });
 
-    await this.notificationRepository.sendEmail({
+    const { messageId } = await this.notificationRepository.sendEmail({
       to: user.email,
       subject: 'Test email from Immich',
       html,
@@ -161,6 +161,8 @@ export class NotificationService {
       replyTo: dto.replyTo || dto.from,
       smtp: dto.transport,
     });
+
+    return { messageId };
   }
 
   async handleUserSignup({ id, tempPassword }: INotifySignupJob) {
@@ -312,10 +314,6 @@ export class NotificationService {
       imageAttachments: data.imageAttachments,
     });
 
-    if (!response) {
-      return JobStatus.FAILED;
-    }
-
     this.logger.log(`Sent mail with id: ${response.messageId} status: ${response.response}`);
 
     return JobStatus.SUCCESS;

server/test/repositories/notification.repository.mock.ts

@@ -4,7 +4,7 @@ import { Mocked } from 'vitest';
 export const newNotificationRepositoryMock = (): Mocked<INotificationRepository> => {
   return {
     renderEmail: vitest.fn(),
-    sendEmail: vitest.fn(),
+    sendEmail: vitest.fn().mockResolvedValue({ messageId: 'message-1' }),
     verifySmtp: vitest.fn(),
   };
 };

web/src/lib/utils/handle-error.ts

@@ -2,9 +2,21 @@ import { isHttpError } from '@immich/sdk';
 import { notificationController, NotificationType } from '../components/shared-components/notification/notification';
 
 export function getServerErrorMessage(error: unknown) {
-  if (isHttpError(error)) {
-    return error.data?.message || error.message;
+  if (!isHttpError(error)) {
+    return;
   }
+
+  // errors for endpoints without return types aren't parsed as json
+  let data = error.data;
+  if (typeof data === 'string') {
+    try {
+      data = JSON.parse(data);
+    } catch {
+      // Not a JSON string
+    }
+  }
+
+  return data?.message || error.message;
 }
 
 export function handleError(error: unknown, message: string) {