fix: fixes in javascript transformation

rudderlabs · Jun 18, 2024 · 67e9277 · 67e9277
1 parent a80d403
commit 67e9277
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 50 deletions.
diff --git a/src/util/customTransformer-v1.js b/src/util/customTransformer-v1.js
@@ -63,6 +63,8 @@ async function userTransformHandlerV1(
     userTransformation.code,
     libraryVersionIds,
     userTransformation.versionId,
+    userTransformation.id,
+    userTransformation.workspaceId || events[0].metadata?.workspaceId,
     credentials,
     userTransformation.secrets || {},
     testMode,

diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js
@@ -289,15 +289,15 @@ async function userTransformHandler(
       });
       const credentialsMap = {};
       if (testMode === false) {
-        events[0]?.credentials?.forEach((cred) => {
-          credentialsMap[cred.key] = cred.value;
+        (events[0]?.credentials || []).forEach((cred) => {
+          credentialsMap[cred.Key] = cred.Value;
         });
       } else {
-        credentials?.forEach((cred) => {
-          credentialsMap[cred.key] = cred.value;
+        (credentials || []).forEach((cred) => {
+          credentialsMap[cred.Key] = cred.Value;
         });
         events.forEach((ev) => {
-          if (isNil(ev?.credentials)) {
+          if (isNil(ev.credentials)) {
             ev.credentials = credentials;
           }
         });

diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js
@@ -30,7 +30,16 @@ async function loadModule(isolateInternal, contextInternal, moduleName, moduleCo
   return module;
 }
 
-async function createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode) {
+async function createIvm(
+  code,
+  libraryVersionIds,
+  versionId,
+  transformationId,
+  workspaceId,
+  credentials,
+  secrets,
+  testMode,
+) {
   const createIvmStartTime = new Date();
   const logs = [];
   const libraries = await Promise.all(
@@ -243,16 +252,12 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret
     }),
   );
 
-  await jail.set('_credential', function (...args) {
+  await jail.set('_credential', function (key) {
     if (_.isNil(credentials) || !_.isObject(credentials)) {
-      logger.error('Error fetching credentials map');
-      stats.increment('credential_error', { versionId });
+      logger.error('Error fetching credentials map', versionId);
+      stats.increment('credential_error', { transformationId, workspaceId });
       return undefined;
     }
-    const key = args[0][0];
-    if (_.isNil(key)) {
-      throw new Error('Key should be valid and defined');
-    }
     return credentials[key];
   });
 
@@ -337,9 +342,11 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret
       let credential = _credential;
       delete _credential;
       global.credential = function(...args) {
-        return credential([
-          ...args.map(arg => new ivm.ExternalCopy(arg).copyInto())
-        ]);
+        const key = args[0];
+        if (key === null || key === undefined) {
+          throw new Error('Key should be valid and defined'+ JSON.stringify(args));
+        }
+        return credential(new ivm.ExternalCopy(key).copyInto());
       };
 
       return new ivm.Reference(function forwardMainPromise(
@@ -432,10 +439,28 @@ async function compileUserLibrary(code) {
   return evaluateModule(isolate, context, code);
 }
 
-async function getFactory(code, libraryVersionIds, versionId, credentials, secrets, testMode) {
+async function getFactory(
+  code,
+  libraryVersionIds,
+  transformationId,
+  workspaceId,
+  versionId,
+  credentials,
+  secrets,
+  testMode,
+) {
   const factory = {
     create: async () => {
-      return createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode);
+      return createIvm(
+        code,
+        libraryVersionIds,
+        versionId,
+        transformationId,
+        workspaceId,
+        credentials,
+        secrets,
+        testMode,
+      );
     },
     destroy: async (client) => {
       client.fnRef.release();

diff --git a/src/util/prometheus.js b/src/util/prometheus.js
@@ -570,10 +570,10 @@ class Prometheus {
         ],
       },
       {
-        name: 'credentials_error',
+        name: 'credential_error',
         help: 'Error in fetching credentials count',
         type: 'counter',
-        labelNames: ['versionId'],
+        labelNames: ['transformationId', 'workspaceId'],
       },
 
       // Gauges

diff --git a/test/__tests__/data/user_transformation_input_credentials.json b/test/__tests__/data/user_transformation_input_credentials.json
@@ -51,14 +51,16 @@
     },
     "credentials": [
       {
-        "key": "key1",
-        "value": "value1",
-        "isPublic": true
+        "Id": "id1",
+        "Key": "key1",
+        "Value": "value1",
+        "IsSecret": false
       },
       {
-        "key": "key2",
-        "value": "value2",
-        "isPublic": true
+        "Id": "id2",
+        "Key": "key2",
+        "Value": "value2",
+        "IsSecret": true
       }
     ]
   }

diff --git a/test/__tests__/user_transformation.integration.test.js b/test/__tests__/user_transformation.integration.test.js
@@ -159,14 +159,14 @@ describe("Function invocation & creation tests", () => {
       versionId,
       [],
       trRevCode,
-      null,
+      [],
       true
     );
     expect(response).toEqual(outputEvents);
 
     // Test with language python; should return same output
     trRevCode = contructTrRevCode(versionId, 'python');
-    response = await userTransformHandler(inputEvents, versionId, [], trRevCode, null, true);
+    response = await userTransformHandler(inputEvents, versionId, [], trRevCode, [], true);
     expect(response).toEqual(outputEvents);
   });
 
@@ -185,7 +185,7 @@ describe("Function invocation & creation tests", () => {
       });
 
     await expect(async () => {
-      await userTransformHandler(inputEvents, respBody.versionId, null, []);
+      await userTransformHandler(inputEvents, respBody.versionId, []);
     }).rejects.toThrow(RetryRequestError);
 
     // If function is not found, it will be created

diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js
@@ -1073,7 +1073,7 @@ describe("User transformation", () => {
       trRevCode.versionId,
       [libraryVersionId],
       trRevCode,
-      null,
+      [],
       true
     );
 
@@ -1110,7 +1110,7 @@ describe("User transformation", () => {
       trRevCode.versionId,
       [],
       trRevCode,
-      null,
+      [],
       true
     );
     expect(output).toEqual(expectedData);
@@ -1128,7 +1128,7 @@ describe("User transformation", () => {
         name,
         code: `
           export function transformEvent(event, metadata) {
-              event.credentialValue = credential("key1");
+              event.credentialValue = credential('key1');
               return event;
             }
             `
@@ -1174,7 +1174,7 @@ describe("User transformation", () => {
       expect(output[0].error).toMatch(/Key should be valid and defined/);
     });
 
-    it(`Simple ${name} Test with credentials with multiple arguements for codeVersion 1`, async () => {
+    it(`Simple ${name} Test with credentials with multiple arguments for codeVersion 1`, async () => {
       const versionId = randomID();
 
       const inputData = require(`./data/${integration}_input_credentials.json`);
@@ -1214,7 +1214,10 @@ describe("User transformation", () => {
         name,
         code: `
           export function transformEvent(event, metadata) {
-              event.credentialValue = credential(1);
+              event.credentialValueForNumkey = credential(1);
+              event.credentialValueForBoolkey = credential(true);
+              event.credentialValueForArraykey = credential([]);
+              event.credentialValueForObjkey = credential({});
               return event;
             }
             `
@@ -1229,7 +1232,10 @@ describe("User transformation", () => {
       expect(fetch).toHaveBeenCalledWith(
         `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}`
       );
-      expect(output[0].transformedEvent.credentialValue).toEqual(undefined);
+      expect(output[0].transformedEvent.credentialValueForNumkey).toEqual(undefined);
+      expect(output[0].transformedEvent.credentialValueForBoolkey).toEqual(undefined);
+      expect(output[0].transformedEvent.credentialValueForArraykey).toEqual(undefined);
+      expect(output[0].transformedEvent.credentialValueForObjkey).toEqual(undefined);
     });
 
     it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => {
@@ -1707,7 +1713,7 @@ describe("Python transformations", () => {
       trRevCode.versionId,
       [],
       trRevCode,
-      null,
+      [],
       true
     );
     expect(output).toEqual(outputData);
@@ -1753,7 +1759,7 @@ describe("Python transformations", () => {
       trRevCode.versionId,
       Object.values(importNameLibraryVersionIdsMap),
       trRevCode,
-      null,
+      [],
       true
     );
     expect(output).toEqual(outputData);
@@ -1795,7 +1801,7 @@ describe("Python transformations", () => {
       trRevCode.versionId,
       Object.values(importNameLibraryVersionIdsMap),
       trRevCode,
-      null,
+      [],
       true
     );
     expect(output).toEqual(outputData);

diff --git a/test/__tests__/user_transformation_fetch.test.js b/test/__tests__/user_transformation_fetch.test.js
@@ -43,7 +43,7 @@ describe("User transformation fetch tests", () => {
           `
     };
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
     expect(output).toEqual({
       logs: [],
       transformedEvents: expectedData.map(ev => (ev.transformedEvent))
@@ -67,7 +67,7 @@ describe("User transformation fetch tests", () => {
     };
     const errMsg = "ERROR";
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(0);
     output.transformedEvents.forEach(ev => {
@@ -92,7 +92,7 @@ describe("User transformation fetch tests", () => {
     const errMsg = "ERROR";
 
     mockResolver.mockResolvedValue([ '127.0.0.1' ]);
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
     expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
@@ -122,7 +122,7 @@ describe("User transformation fetch tests", () => {
     };
     const errMsg = "invalid url, localhost requests are not allowed";
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(0);
     output.transformedEvents.forEach(ev => {
@@ -152,7 +152,7 @@ describe("User transformation fetch tests", () => {
     const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: unable to resolve IP address for abc.xyz.com";
 
     mockResolver.mockRejectedValue('invalid host');
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
     expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
@@ -182,7 +182,7 @@ describe("User transformation fetch tests", () => {
     };
     const errMsg = "invalid protocol, only http and https are supported";
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     output.transformedEvents.forEach(ev => {
       expect(ev.errMsg).toEqual(errMsg);
@@ -204,7 +204,7 @@ describe("User transformation fetch tests", () => {
       versionId,
     };
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
     expect(output).toEqual({
       logs: [],
       transformedEvents: expectedData.map(ev => (ev.transformedEvent))
@@ -226,7 +226,7 @@ describe("User transformation fetch tests", () => {
     };
     const errMsg = "ERROR";
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(0);
     output.transformedEvents.forEach(ev => {
@@ -250,7 +250,7 @@ describe("User transformation fetch tests", () => {
     const errMsg = "ERROR";
 
     mockResolver.mockRejectedValue('invalid host');
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
     expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
@@ -279,7 +279,7 @@ describe("User transformation fetch tests", () => {
     };
     const errMsg = "invalid url, localhost requests are not allowed";
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(0);
     output.transformedEvents.forEach(ev => {
@@ -308,7 +308,7 @@ describe("User transformation fetch tests", () => {
     const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: cannot use 127.0.0.1 as IP address";
 
     mockResolver.mockResolvedValue(['3.122.122.122', '127.0.0.1']);
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
     expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
@@ -337,7 +337,7 @@ describe("User transformation fetch tests", () => {
     };
     const errMsg = "fetch url is required";
 
-    const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
+    const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
 
     output.transformedEvents.forEach(ev => {
       expect(ev.errMsg).toEqual(errMsg);