Skip to content

Investigate data locking when uploading attachments #3854

@olemartinorg

Description

@olemartinorg

In Altinn/altinn-studio#16966 (comment), coderabbit noticed something that worries me. We should investigate this and perhaps add a cypress test for verifying that locks are always released when uploading fails:

If anything between acquiring the lock and the final unlock(updatedData) throws (for example setAttachmentsInDataModel or uploadFinished), the function exits early and the FD lock stays engaged. Once that happens every subsequent form-data mutation will queue behind an unlock that never comes, effectively bricking uploads for the rest of the session. Please wrap the body in a try/finally so we always release the lock.

-            const { unlock } = await lock();
-            const results: AttachmentUploadResult[] = [];
-
-            const updatedData: FDActionResult = { updatedDataModels: {}, updatedValidationIssues: {} };
-
-            for (const { file, temporaryId } of fullAction.files) {
-              const { baseComponentId } = splitDashedKey(action.nodeId);
-              try {
-                const reply = await uploadAttachment({
-                  dataTypeId: baseComponentId,
-                  file,
-                });
-                results.push({ temporaryId, newDataElementId: reply.newDataElementId });
-
-                updatedData.instance = reply.instance;
-                updatedData.updatedDataModels = {
-                  ...updatedData.updatedDataModels,
-                  ...dataModelPairsToObject(reply.newDataModels),
-                };
-                updatedData.updatedValidationIssues = {
-                  ...updatedData.updatedValidationIssues,
-                  ...backendValidationIssueGroupListToObject(reply.validationIssues),
-                };
-              } catch (error) {
-                results.push({ temporaryId, error });
-              }
-            }
-            setAttachmentsInDataModel(
-              results.filter(isAttachmentUploadSuccess).map(({ newDataElementId }) => newDataElementId),
-              action.dataModelBindings,
-            );
-            uploadFinished(fullAction, results);
-            unlock(updatedData);
+            const { unlock } = await lock();
+            const results: AttachmentUploadResult[] = [];
+
+            const updatedData: FDActionResult = { updatedDataModels: {}, updatedValidationIssues: {} };
+
+            try {
+              for (const { file, temporaryId } of fullAction.files) {
+                const { baseComponentId } = splitDashedKey(action.nodeId);
+                try {
+                  const reply = await uploadAttachment({
+                    dataTypeId: baseComponentId,
+                    file,
+                  });
+                  results.push({ temporaryId, newDataElementId: reply.newDataElementId });
+
+                  updatedData.instance = reply.instance;
+                  updatedData.updatedDataModels = {
+                    ...updatedData.updatedDataModels,
+                    ...dataModelPairsToObject(reply.newDataModels),
+                  };
+                  updatedData.updatedValidationIssues = {
+                    ...updatedData.updatedValidationIssues,
+                    ...backendValidationIssueGroupListToObject(reply.validationIssues),
+                  };
+                } catch (error) {
+                  results.push({ temporaryId, error });
+                }
+              }
+              setAttachmentsInDataModel(
+                results.filter(isAttachmentUploadSuccess).map(({ newDataElementId }) => newDataElementId),
+                action.dataModelBindings,
+              );
+              uploadFinished(fullAction, results);
+            } finally {
+              unlock(updatedData);
+            }

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions