error handling in securefiles (#487)

* errror handling in securefiles

* updating version of securefiles common package

* adding test cases

* changes in package-lock.json

Author: Deekshitha2812

common-npm-packages/securefiles-common/Tests/L0.ts

@@ -23,29 +23,59 @@ const tmAnswers = {
     }
 }
 
-class AgentAPI {
-    downloadSecureFile() {
-        const rs = new Readable();
-        rs._read = () => {};
-        rs.push('data');
+function createMockStream(options: {
+    statusCode?: number;
+    statusMessage?: string;
+    contentType?: string;
+    data?: string;
+    emitError?: Error;
+} = {}) {
+    const {
+        statusCode = 200,
+        statusMessage = 'OK',
+        contentType = 'application/octet-stream',
+        data = 'data',
+        emitError,
+    } = options;
+
+    const rs = new Readable();
+    rs._read = () => {};
+    (rs as any).statusCode = statusCode;
+    (rs as any).statusMessage = statusMessage;
+    (rs as any).headers = { 'content-type': contentType };
+    
+    if (emitError) {
+        setTimeout(() => {
+            rs.emit('error', emitError);
+        }, 10);
+    } else {
+        if (data) rs.push(data);
         rs.push(null);
-        return rs;
     }
+    
+    return rs;
 }
 
-class WebApi {
-    getTaskAgentApi() {
-        return new Promise((resolve) => {
-            resolve(new AgentAPI());
-        });
+// Helper function to create a complete node API mock
+function createNodeApiMock(streamOptions?: Parameters<typeof createMockStream>[0]) {
+    class MockAgentAPI {
+        downloadSecureFile() {
+            return Promise.resolve(createMockStream(streamOptions));
+        }
     }
-}
 
-export const nodeapiMock = {
-    WebApi,
-    getPersonalAccessTokenHandler() {
-        return {} as IRequestHandler;
+    class MockWebApi {
+        getTaskAgentApi() {
+            return Promise.resolve(new MockAgentAPI());
+        }
     }
+
+    return {
+        WebApi: MockWebApi,
+        getPersonalAccessTokenHandler() {
+            return {} as IRequestHandler;
+        }
+    };
 }
 
 export const fsMock = {
@@ -105,6 +135,12 @@ describe("securefiles-common package suites", function() {
     });
 
     it("Check downloadSecureFile", async() => {
+        const nodeapiMock = createNodeApiMock({
+            statusCode: 200,
+            statusMessage: 'OK',
+            contentType: 'application/octet-stream'
+        });
+        
         registerMock("azure-devops-node-api", nodeapiMock);
         registerMock("fs", fsMock);
         const secureFiles = require("../securefiles-common");
@@ -127,4 +163,49 @@ describe("securefiles-common package suites", function() {
         const pseudoResolvedPath = tlClone.resolve(tlClone.getVariable("Agent.TempDirectory"), tlClone.getSecureFileName(secureFileId));
         strictEqual(resolvedPath, pseudoResolvedPath, `Resolved path "${resolvedPath}" should be equal to "${pseudoResolvedPath}"`);
     });
+
+    it("Should handle HTTP error responses", async() => {
+        const errorNodeapiMock = createNodeApiMock({
+            statusCode: 500,
+            statusMessage: 'Internal Server Error',
+            contentType: 'application/json',
+            data: '',
+        });
+
+        registerMock("azure-devops-node-api", errorNodeapiMock);
+        registerMock("fs", fsMock);
+        const secureFiles = require("../securefiles-common");
+        const secureFileHelpers = new secureFiles.SecureFileHelpers();
+        
+        try {
+            await secureFileHelpers.downloadSecureFile(secureFileId);
+            throw new Error("Expected error was not thrown");
+        } catch (error) {
+            strictEqual(error.message.includes("HTTP 500"), true, "Should contain HTTP error status");
+            strictEqual(error.message.includes("Internal Server Error"), true, "Should contain HTTP error message");
+        }
+    });
+
+    it("Should handle stream errors during download", async() => {
+        const streamErrorNodeapiMock = createNodeApiMock({
+            statusCode: 200,
+            statusMessage: 'OK',
+            contentType: 'application/octet-stream',
+            emitError: new Error('Network connection lost')
+        });
+
+        registerMock("azure-devops-node-api", streamErrorNodeapiMock);
+        registerMock("fs", fsMock);
+
+        const secureFiles = require("../securefiles-common");
+        const secureFileHelpers = new secureFiles.SecureFileHelpers();
+        
+        try {
+            await secureFileHelpers.downloadSecureFile(secureFileId);
+            throw new Error("Expected error was not thrown");
+        } catch (error) {
+            strictEqual(error.message.includes("Failed to download secure file"), true, "Should handle stream errors");
+            strictEqual(error.message.includes("Network connection lost"), true, "Should include original error message");
+        }
+    });
 });

common-npm-packages/securefiles-common/package-lock.json

@@ -1,6 +1,6 @@
 {
     "name": "azure-pipelines-tasks-securefiles-common",
-    "version": "2.262.0",
+    "version": "2.263.0",
     "description": "Azure Pipelines tasks SecureFiles Common",
     "main": "securefiles-common.js",
     "scripts": {

common-npm-packages/securefiles-common/package.json

@@ -34,26 +34,48 @@ export class SecureFileHelpers {
      * @param secureFileId
      */
     async downloadSecureFile(secureFileId: string): Promise<string> {
-        const tempDownloadPath: string = this.getSecureFileTempDownloadPath(secureFileId);
 
+        const tempDownloadPath: string = this.getSecureFileTempDownloadPath(secureFileId);
         tl.debug('Downloading secure file contents to: ' + tempDownloadPath);
-        const file: NodeJS.WritableStream = fs.createWriteStream(tempDownloadPath);
 
         const agentApi = await this.serverConnection.getTaskAgentApi();
-
         const ticket = tl.getSecureFileTicket(secureFileId);
         if (!ticket) {
             // Workaround bug #7491. tl.loc only works if the consuming tasks define the resource string.
             throw new Error(`Download ticket for SecureFileId ${secureFileId} not found.`);
         }
 
-        const stream = (await agentApi.downloadSecureFile(
-            tl.getVariable('SYSTEM.TEAMPROJECT'), secureFileId, ticket, false)).pipe(file);
+        tl.debug(`Starting secure file download for SecureFileId: ${secureFileId}`);
+        const response = await agentApi.downloadSecureFile(tl.getVariable('SYSTEM.TEAMPROJECT'), secureFileId, ticket, false);
 
-        const defer = Q.defer();
+        const defer = Q.defer<void>();
+        response.on('error', (err: Error) => {
+            defer.reject(new Error(`Failed to download secure file. ${err.message}`));
+        });
+        const httpResponse = response as any;
+        tl.debug(`HTTP Status: ${httpResponse.statusCode} ${httpResponse.statusMessage}`);
+        tl.debug(`Content-Type: ${httpResponse.headers['content-type'] || 'unknown'}`);
+
+        if (httpResponse.statusCode && httpResponse.statusCode != 200) {
+            let errorBody = '';
+            httpResponse.on('data', (chunk: Buffer) => {
+                errorBody += chunk.toString();
+            });
+            httpResponse.on('end', () => {
+                defer.reject(new Error(`Failed to download secure file. HTTP ${httpResponse.statusCode}: ${httpResponse.statusMessage}. Error content: ${errorBody}`));
+            });
+        }
+
+        
+        const file: NodeJS.WritableStream = fs.createWriteStream(tempDownloadPath);
+        const stream = response.pipe(file);
+        
         stream.on('finish', () => {
             defer.resolve();
         });
+        stream.on('error', (err: Error) => {
+            defer.reject(new Error(`Error writing secure file to disk. ${err.message}`));
+        });
         await defer.promise;
         tl.debug('Downloaded secure file contents to: ' + tempDownloadPath);
         return tempDownloadPath;