From cc9669830094a8681633a4476da79a364f0f7336 Mon Sep 17 00:00:00 2001
From: Max <maxharz@gmail.com>
Date: Fri, 20 Jun 2025 20:32:00 +0200
Subject: [PATCH] Put message creation in a transaction with file uploads to
 avoid empty messages (#8694)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Wolfgang Huß <wolle.huss@pjannto.com>
---
 .../src/graphql/resolvers/messages.spec.ts    | 49 ++++++++++++
 backend/src/graphql/resolvers/messages.ts     | 76 +++++++++----------
 2 files changed, 83 insertions(+), 42 deletions(-)

diff --git a/backend/src/graphql/resolvers/messages.spec.ts b/backend/src/graphql/resolvers/messages.spec.ts
index f0479432b..d51a29993 100644
--- a/backend/src/graphql/resolvers/messages.spec.ts
+++ b/backend/src/graphql/resolvers/messages.spec.ts
@@ -283,6 +283,55 @@ describe('Message', () => {
           })
         })
 
+        describe('user sends file, but upload goes wrong', () => {
+          const file1 = Readable.from('file1')
+          const upload1 = new Upload()
+          upload1.resolve({
+            createReadStream: () => file1,
+            stream: file1,
+            filename: 'file1',
+            encoding: '7bit',
+            mimetype: 'application/json',
+          })
+          const upload2 = new Upload()
+          upload2.resolve(new Error('Upload failed'))
+
+          it('no message is created', async () => {
+            await expect(
+              mutate({
+                mutation: CreateMessage,
+                variables: {
+                  roomId,
+                  content: 'A message which should not be created',
+                  files: [
+                    { upload: upload1, name: 'test1', type: 'application/json' },
+                    { upload: upload2, name: 'test2', type: 'image/png' },
+                  ],
+                },
+              }),
+            ).resolves.toMatchObject({
+              errors: {},
+              data: {
+                CreateMessage: null,
+              },
+            })
+
+            await expect(
+              query({
+                query: Message,
+                variables: {
+                  roomId,
+                },
+              }),
+            ).resolves.toMatchObject({
+              errors: undefined,
+              data: {
+                Message: [],
+              },
+            })
+          })
+        })
+
         describe('user does not chat in room', () => {
           beforeEach(async () => {
             authenticatedUser = await notChattingUser.toJson()
diff --git a/backend/src/graphql/resolvers/messages.ts b/backend/src/graphql/resolvers/messages.ts
index bc831c671..1c588ba5d 100644
--- a/backend/src/graphql/resolvers/messages.ts
+++ b/backend/src/graphql/resolvers/messages.ts
@@ -76,9 +76,12 @@ export default {
       const {
         user: { id: currentUserId },
       } = context
+
       const session = context.driver.session()
-      const writeTxResultPromise = session.writeTransaction(async (transaction) => {
-        const createMessageCypher = `
+
+      try {
+        const writeTxResultPromise = session.writeTransaction(async (transaction) => {
+          const createMessageCypher = `
           MATCH (currentUser:User { id: $currentUserId })-[:CHATS_IN]->(room:Room { id: $roomId })
           OPTIONAL MATCH (currentUser)-[:AVATAR_IMAGE]->(image:Image)
           OPTIONAL MATCH (m:Message)-[:INSIDE]->(room)
@@ -105,53 +108,42 @@ export default {
             date: message.createdAt
           }
         `
-        const createMessageTxResponse = await transaction.run(createMessageCypher, {
-          currentUserId,
-          roomId,
-          content,
-        })
+          const createMessageTxResponse = await transaction.run(createMessageCypher, {
+            currentUserId,
+            roomId,
+            content,
+          })
 
-        const [message] = await createMessageTxResponse.records.map((record) =>
-          record.get('message'),
-        )
+          const [message] = await createMessageTxResponse.records.map((record) =>
+            record.get('message'),
+          )
 
-        return message
-      })
-      try {
-        // We cannot combine the query above with the attachments, since you need the resource for matching
-        const message = await writeTxResultPromise
-
-        // this is the case if the room doesn't exist - requires refactoring for implicit rooms
-        if (!message) {
-          return null
-        }
-
-        const session = context.driver.session()
-        const writeFilesPromise = session.writeTransaction(async (transaction) => {
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-          const atns: any[] = []
-
-          if (!isS3configured(CONFIG)) {
-            return atns
+          // this is the case if the room doesn't exist - requires refactoring for implicit rooms
+          if (!message) {
+            return null
           }
 
-          for await (const file of files) {
-            const atn = await attachments(CONFIG).add(
-              message,
-              'ATTACHMENT',
-              file,
-              {},
-              {
-                transaction,
-              },
-            )
-            atns.push(atn)
+          const atns: File[] = []
+
+          if (isS3configured(CONFIG)) {
+            for await (const file of files) {
+              const atn = await attachments(CONFIG).add(
+                message,
+                'ATTACHMENT',
+                file,
+                {},
+                {
+                  transaction,
+                },
+              )
+              atns.push(atn)
+            }
           }
-          return atns
+
+          return { ...message, files: atns }
         })
 
-        const atns = await writeFilesPromise
-        return { ...message, files: atns }
+        return await writeTxResultPromise
       } catch (error) {
         throw new Error(error)
       } finally {