🐛 [in brackets] back: immediately start streaming downloading of zip #783 (#788)

When downloading a folder with a great many entries below, time to first
byte can cause a tcp disconnection for timeout by a reverse proxy, this
starts the download asap, and ignores unreadable entries because of AV
or rights. It tries to generate a valid zip even if some entries are
missing for those reasons

tdrive/backend/node/src/services/documents/services/index.ts

@@ -1505,10 +1505,17 @@ export class DocumentsService {
       if (token) throw new CrudException("Invalid token", 401);
     }
   };
-
+  /**
+   *
+   * @param id DriveItemID to download. If it's a folder, a zip file will be returned
+   * @param versionId Optional specific version to download
+   * @param {Function} beginArchiveTransmit If a folder,
+   *   called when the zip file can begin streaming, otherwise not called at all.
+   */
   download = async (
     id: string,
     versionId: string | null,
+    beginArchiveTransmit: (readable: archiver.Archiver) => void,
     context: DriveExecutionContext,
   ): Promise<{
     archive?: archiver.Archiver;
@@ -1522,7 +1529,7 @@ export class DocumentsService {
     const item = await this.get(id, null, context);
 
     if (item.item.is_directory) {
-      return { archive: await this.createZip([id], context) };
+      return { archive: await this.createZip([id], beginArchiveTransmit, context) };
     }
 
     let version = item.item.last_version_cache;
@@ -1541,11 +1548,13 @@ export class DocumentsService {
    * Creates a zip archive containing the drive items.
    *
    * @param {string[]} ids - the drive item list
+   * @param {Function} beginArchiveTransmit - Called when the zip file can begin streaming
    * @param {DriveExecutionContext} context - the execution context
    * @returns {Promise<archiver.Archiver>} the created archive.
    */
   createZip = async (
     ids: string[] = [],
+    beginArchiveTransmit: (archive: archiver.Archiver) => void,
     context: DriveExecutionContext,
   ): Promise<archiver.Archiver> => {
     if (!context) {
@@ -1561,19 +1570,31 @@ export class DocumentsService {
       this.logger.error("error while creating ZIP file: ", error);
     });
 
+    let didBeginTransmission = false;
     for (const id of ids) {
       if (!(await checkAccess(id, null, "read", this.repository, context))) {
-        this.logger.warn(`not enough permissions to download ${id}, skipping`);
-        return;
+        this.logger.warn({ id }, `not enough permissions to download ${id}, skipping`);
+        continue;
       }
 
       try {
-        await addDriveItemToArchive(id, null, archive, this.repository, context);
-      } catch (error) {
-        console.error(error);
-        this.logger.warn("failed to add item to archive", error);
+        await addDriveItemToArchive(
+          id,
+          null,
+          archive,
+          archive => {
+            if (didBeginTransmission) return;
+            didBeginTransmission = true;
+            beginArchiveTransmit(archive);
+          },
+          this.repository,
+          context,
+        );
+      } catch (err) {
+        this.logger.warn({ err, id }, "failed to add item to archive");
       }
     }
+    if (!didBeginTransmission) beginArchiveTransmit(archive);
 
     //TODO[ASH] why do we need this call??
     archive.finalize();

tdrive/backend/node/src/services/documents/utils.ts

@@ -345,6 +345,9 @@ export const addDriveItemToArchive = async (
   id: string,
   entity: DriveFile | null,
   archive: archiver.Archiver,
+  beginArchiveTransmit:
+    | "ONLY_SET_THIS_VALUE_IF_ALREADY_CALLED"
+    | ((archive: archiver.Archiver) => void),
   repository: Repository<DriveFile>,
   context: CompanyExecutionContext,
   prefix?: string,
@@ -368,6 +371,10 @@ export const addDriveItemToArchive = async (
     }
 
     archive.append(file.file, { name: item.name, prefix: prefix ?? "" });
+    if (beginArchiveTransmit !== "ONLY_SET_THIS_VALUE_IF_ALREADY_CALLED") {
+      beginArchiveTransmit(archive);
+      beginArchiveTransmit = "ONLY_SET_THIS_VALUE_IF_ALREADY_CALLED";
+    }
     return;
   } else {
     let nextPage = "";
@@ -384,6 +391,7 @@ export const addDriveItemToArchive = async (
           child.id,
           child,
           archive,
+          beginArchiveTransmit,
           repository,
           context,
           `${prefix || ""}${item.name}/`,

tdrive/backend/node/src/services/documents/web/controllers/documents.ts

@@ -551,21 +551,21 @@ export class DocumentsController {
       const archiveOrFile = await globalResolver.services.documents.documents.download(
         id,
         versionId,
+        archive => {
+          archive.on("finish", () => {
+            response.status(200);
+          });
+
+          archive.on("error", () => {
+            response.internalServerError();
+          });
+
+          archive.pipe(response.raw);
+        },
         context,
       );
 
       if (archiveOrFile.archive) {
-        const archive = archiveOrFile.archive;
-
-        archive.on("finish", () => {
-          response.status(200);
-        });
-
-        archive.on("error", () => {
-          response.internalServerError();
-        });
-
-        archive.pipe(response.raw);
         return response;
       } else if (archiveOrFile.file) {
         const data = archiveOrFile.file;
@@ -623,22 +623,27 @@ export class DocumentsController {
     }
 
     try {
-      const archive = await globalResolver.services.documents.documents.createZip(ids, context);
-      reply.raw.setHeader(
-        "content-disposition",
-        formatAttachmentContentDispositionHeader("twake_drive.zip"),
+      await globalResolver.services.documents.documents.createZip(
+        ids,
+        archive => {
+          reply.raw.setHeader(
+            "content-disposition",
+            formatAttachmentContentDispositionHeader("twake_drive.zip"),
+          );
+
+          archive.on("finish", () => {
+            reply.status(200);
+          });
+
+          archive.on("error", () => {
+            reply.internalServerError();
+          });
+
+          archive.pipe(reply.raw);
+        },
+        context,
       );
 
-      archive.on("finish", () => {
-        reply.status(200);
-      });
-
-      archive.on("error", () => {
-        reply.internalServerError();
-      });
-
-      archive.pipe(reply.raw);
-
       return reply;
     } catch (error) {
       logger.error({ error: `${error}` }, "failed to send zip file");