fix: address code review findings for MMS attachments

- Use deterministic reverse map for ContentTypeFromExtension (I-2)
- Initialize attachmentURLs to []string{} to avoid null in JSON (I-3)
- Distinguish 404 vs 500 in download handler with ErrAttachmentNotFound (I-4)
- Remove unused stacktrace import from memory storage

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: AchoArnold

api/pkg/handlers/attachment_handler.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -68,7 +69,10 @@ func (h *AttachmentHandler) GetAttachment(c *fiber.Ctx) error {
 	if err != nil {
 		msg := fmt.Sprintf("cannot download attachment from path [%s]", path)
 		ctxLogger.Warn(stacktrace.Propagate(err, msg))
-		return h.responseNotFound(c, "attachment not found")
+		if errors.Is(err, repositories.ErrAttachmentNotFound) {
+			return h.responseNotFound(c, "attachment not found")
+		}
+		return h.responseInternalServerError(c)
 	}
 
 	ext := filepath.Ext(filename)

api/pkg/repositories/attachment_storage.go

@@ -34,6 +34,22 @@ var contentTypeExtensions = map[string]string{
 	"text/x-vcard":    ".vcf",
 }
 
+// extensionContentTypes is the reverse map from file extensions to canonical MIME types
+var extensionContentTypes = map[string]string{
+	".jpg":  "image/jpeg",
+	".png":  "image/png",
+	".gif":  "image/gif",
+	".webp": "image/webp",
+	".bmp":  "image/bmp",
+	".mp4":  "video/mp4",
+	".3gp":  "video/3gpp",
+	".mp3":  "audio/mpeg",
+	".ogg":  "audio/ogg",
+	".amr":  "audio/amr",
+	".pdf":  "application/pdf",
+	".vcf":  "text/vcard",
+}
+
 // AllowedContentTypes returns the set of allowed MIME types for attachments
 func AllowedContentTypes() map[string]bool {
 	allowed := make(map[string]bool, len(contentTypeExtensions))
@@ -55,14 +71,15 @@ func ExtensionFromContentType(contentType string) string {
 // ContentTypeFromExtension returns the MIME content type for a file extension.
 // Returns "application/octet-stream" if the extension is not recognized.
 func ContentTypeFromExtension(ext string) string {
-	for ct, e := range contentTypeExtensions {
-		if e == ext {
-			return ct
-		}
+	if ct, ok := extensionContentTypes[ext]; ok {
+		return ct
 	}
 	return "application/octet-stream"
 }
 
+// ErrAttachmentNotFound is returned when an attachment is not found in storage
+var ErrAttachmentNotFound = fmt.Errorf("attachment not found")
+
 // SanitizeFilename removes path separators and traversal sequences from a filename.
 // Returns "attachment-{index}" if the sanitized name is empty.
 func SanitizeFilename(name string, index int) string {

api/pkg/repositories/memory_attachment_storage.go

@@ -6,7 +6,6 @@ import (
 	"sync"
 
 	"github.com/NdoleStudio/httpsms/pkg/telemetry"
-	"github.com/palantir/stacktrace"
 )
 
 // MemoryAttachmentStorage stores attachments in memory
@@ -44,7 +43,7 @@ func (s *MemoryAttachmentStorage) Download(ctx context.Context, path string) ([]
 
 	value, ok := s.data.Load(path)
 	if !ok {
-		return nil, stacktrace.NewError(fmt.Sprintf("attachment not found at path [%s]", path))
+		return nil, ErrAttachmentNotFound
 	}
 	return value.([]byte), nil
 }

api/pkg/services/message_service.go

@@ -323,7 +323,7 @@ func (service *MessageService) ReceiveMessage(ctx context.Context, params *Messa
 	ctxLogger := service.tracer.CtxLogger(service.logger, span)
 
 	messageID := uuid.New()
-	var attachmentURLs []string
+	attachmentURLs := []string{}
 
 	if len(params.Attachments) > 0 {
 		ctxLogger.Info(fmt.Sprintf("uploading [%d] attachments for message [%s]", len(params.Attachments), messageID))