Repository: serverless/dashboard (language: python)

Issue:
SDK: Prevent internal crashes
When SDK processing fails, if possible log error information instead of crashing user Lambda.

- Log error, instead of throwing error on invalid tag values as set for internal tags
- Wrap certain instrumentation parts of HTTP and express with crash prevention
- Wrap registration of captured events with crash prevention

After this is merged and released, it'll be followed up with similar update to AWS Lambda SDK for which changes I have ready locally


Discussion / hints:

Fix patch:
diff --git a/node/packages/aws-lambda-sdk/index.js b/node/packages/aws-lambda-sdk/index.js
index 954cc1ad7..d8eeaca99 100644
--- a/node/packages/aws-lambda-sdk/index.js
+++ b/node/packages/aws-lambda-sdk/index.js
@@ -15,12 +15,16 @@ serverlessSdk.instrumentation.awsSdkV2 = require('./instrumentation/aws-sdk-v2')
 serverlessSdk.instrumentation.awsSdkV3Client = require('./instrumentation/aws-sdk-v3-client');
 
 serverlessSdk._initializeExtension = (options) => {
-  const settings = serverlessSdk._settings;
-  serverlessSdk._settings.disableAwsSdkMonitoring = Boolean(
-    process.env.SLS_DISABLE_AWS_SDK_MONITORING || options.disableAwsSdkMonitoring
-  );
-  if (!settings.disableAwsSdkMonitoring) {
-    require('./lib/instrumentation/aws-sdk').install();
+  try {
+    const settings = serverlessSdk._settings;
+    serverlessSdk._settings.disableAwsSdkMonitoring = Boolean(
+      process.env.SLS_DISABLE_AWS_SDK_MONITORING || options.disableAwsSdkMonitoring
+    );
+    if (!settings.disableAwsSdkMonitoring) {
+      require('./lib/instrumentation/aws-sdk').install();
+    }
+  } catch (error) {
+    serverlessSdk._reportSdkError(error);
   }
 };
 serverlessSdk._isDevMode = Boolean(process.env.SLS_DEV_MODE_ORG_ID);
diff --git a/node/packages/aws-lambda-sdk/instrument/index.js b/node/packages/aws-lambda-sdk/instrument/index.js
index 7631b9997..7fe8ce768 100644
--- a/node/packages/aws-lambda-sdk/instrument/index.js
+++ b/node/packages/aws-lambda-sdk/instrument/index.js
@@ -174,29 +174,28 @@ const closeTrace = async (outcome, outcomeResult) => {
       `${Math.round(Number(process.hrtime.bigint() - endTime) / 1000000)}ms`
     );
   } catch (error) {
-    process._rawDebug(
-      'Fatal Serverless SDK Error: ' +
-        'Please report at https://github.com/serverless/console/issues: ' +
-        'Response handling failed: ',
-      error && (error.stack || error)
-    );
+    serverlessSdk._reportSdkError(error);
     if (!isRootSpanReset) clearRootSpan();
   }
 };
 
-const wrapUnhandledErrorListener = (eventName) => {
-  const [awsListener] = process.listeners(eventName);
-  process.off(eventName, awsListener);
-  process.on(eventName, (error) => {
-    if (isCurrentInvocationResolved) {
-      awsListener(error);
-      return;
-    }
-    closeTrace('error:unhandled', error).finally(() => process.nextTick(() => awsListener(error)));
-  });
-};
-wrapUnhandledErrorListener('uncaughtException');
-wrapUnhandledErrorListener('unhandledRejection');
+if (!process.env.SLS_UNIT_TEST_RUN) {
+  const wrapUnhandledErrorListener = (eventName) => {
+    const [awsListener] = process.listeners(eventName);
+    process.off(eventName, awsListener);
+    process.on(eventName, (error) => {
+      if (isCurrentInvocationResolved) {
+        awsListener(error);
+        return;
+      }
+      closeTrace('error:unhandled', error).finally(() =>
+        process.nextTick(() => awsListener(error))
+      );
+    });
+  };
+  wrapUnhandledErrorListener('uncaughtException');
+  wrapUnhandledErrorListener('unhandledRejection');
+}
 
 module.exports = (originalHandler, options = {}) => {
   ensurePlainFunction(originalHandler, { name: 'originalHandler' });
@@ -255,12 +254,7 @@ module.exports = (originalHandler, options = {}) => {
         `${Math.round(Number(process.hrtime.bigint() - requestStartTime) / 1000000)}ms`
       );
     } catch (error) {
-      process._rawDebug(
-        'Fatal Serverless SDK Error: ' +
-          'Please report at https://github.com/serverless/console/issues: ' +
-          'Request handling failed: ',
-        error && (error.stack || error)
-      );
+      serverlessSdk._reportSdkError(error);
       if (originalDone) contextDone = originalDone;
       return originalHandler(event, context, awsCallback);
     }
diff --git a/node/packages/aws-lambda-sdk/instrument/lib/auto-send-spans.js b/node/packages/aws-lambda-sdk/instrument/lib/auto-send-spans.js
index 00752c157..904fc3514 100644
--- a/node/packages/aws-lambda-sdk/instrument/lib/auto-send-spans.js
+++ b/node/packages/aws-lambda-sdk/instrument/lib/auto-send-spans.js
@@ -21,31 +21,35 @@ const pendingCapturedEvents = [];
 let isScheduled = false;
 let timeoutId = null;
 const sendData = () => {
-  isScheduled = false;
-  clearTimeout(timeoutId);
-  if (!pendingSpans.length && !pendingCapturedEvents.length) return;
-  const payload = {
-    slsTags: {
-      orgId: serverlessSdk.orgId,
-      service: process.env.AWS_LAMBDA_FUNCTION_NAME,
-      sdk: { name: '@serverless/aws-lambda-sdk', version: serverlessSdk.version },
-    },
-    spans: pendingSpans.map((span) => span.toProtobufObject()),
-    events: pendingCapturedEvents.map((capturedEvent) => capturedEvent.toProtobufObject()),
-    customTags: objHasOwnProperty.call(serverlessSdk, '_customTags')
-      ? JSON.stringify(serverlessSdk._customTags)
-      : undefined,
-  };
-  pendingSpans.length = 0;
-  pendingCapturedEvents.length = 0;
-  if (!invocationContextAccessor.value) {
-    // Root span comes with "aws.lambda.*" tags, which require unconditionally requestId
-    // which we don't have if handler crashed at initialization.
-    payload.spans = payload.spans.filter((span) => span.name !== 'aws.lambda');
+  try {
+    isScheduled = false;
+    clearTimeout(timeoutId);
+    if (!pendingSpans.length && !pendingCapturedEvents.length) return;
+    const payload = {
+      slsTags: {
+        orgId: serverlessSdk.orgId,
+        service: process.env.AWS_LAMBDA_FUNCTION_NAME,
+        sdk: { name: '@serverless/aws-lambda-sdk', version: serverlessSdk.version },
+      },
+      spans: pendingSpans.map((span) => span.toProtobufObject()),
+      events: pendingCapturedEvents.map((capturedEvent) => capturedEvent.toProtobufObject()),
+      customTags: objHasOwnProperty.call(serverlessSdk, '_customTags')
+        ? JSON.stringify(serverlessSdk._customTags)
+        : undefined,
+    };
+    pendingSpans.length = 0;
+    pendingCapturedEvents.length = 0;
+    if (!invocationContextAccessor.value) {
+      // Root span comes with "aws.lambda.*" tags, which require unconditionally requestId
+      // which we don't have if handler crashed at initialization.
+      payload.spans = payload.spans.filter((span) => span.name !== 'aws.lambda');
+    }
+    serverlessSdk._deferredTelemetryRequests.push(
+      sendTelemetry('trace', traceProto.TracePayload.encode(payload).finish())
+    );
+  } catch (error) {
+    serverlessSdk._reportSdkError(error);
   }
-  serverlessSdk._deferredTelemetryRequests.push(
-    sendTelemetry('trace', traceProto.TracePayload.encode(payload).finish())
-  );
 };
 
 const scheduleEventually = () => {
diff --git a/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v2.js b/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v2.js
index 25d221598..b9f1532da 100644
--- a/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v2.js
+++ b/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v2.js
@@ -30,49 +30,61 @@ module.exports.install = (Sdk) => {
     return originalPresign.call(this, expires, callback);
   };
   Sdk.Request.prototype.runTo = function runTo(state, done) {
-    // identifier
-    const serviceName =
-      this.service.constructor.serviceIdentifier ||
-      this.service.constructor.__super__.serviceIdentifier;
-    const tagMapper = serviceMapper.get(serviceName);
-    const operationName = this.operation.toLowerCase();
-    const params = this.params;
-    const traceSpan = serverlessSdk._createTraceSpan(`aws.sdk.${serviceName}.${operationName}`, {
-      tags: {
-        'aws.sdk.region': this.service.config.region,
-        'aws.sdk.signature_version': this.service.config.signatureVersion,
-        'aws.sdk.service': serviceName,
-        'aws.sdk.operation': operationName,
-      },
-      input: shouldMonitorRequestResponse ? safeStringify(params) : null,
-    });
-    if (tagMapper && tagMapper.params) tagMapper.params(traceSpan, params);
-    let wasCompleted = false;
-    this.on('complete', (response) => {
-      if (wasCompleted) {
-        console.warn({
-          source: 'serverlessSdk',
-          message:
-            'Detected doubled handling for same AWS SDK request. ' +
-            'It may happen if for the same request both callback and promise resolution ' +
-            'is requested. Internally it creates two AWS SDK calls so such design should be avoided.\n',
-          code: 'AWS_SDK_DOUBLE_RESOLUTION',
-        });
-        return;
-      }
-      wasCompleted = true;
-      if (response.requestId) traceSpan.tags.set('aws.sdk.request_id', response.requestId);
-      if (response.error) {
-        // Fallback to error.name, as there are cases when AWS SDK returns error with no message:
-        // https://github.com/aws/aws-sdk-js/issues/4330
-        traceSpan.tags.set('aws.sdk.error', response.error.message || response.error.name);
-      } else {
-        if (shouldMonitorRequestResponse) traceSpan.output = safeStringify(response.data);
-        if (tagMapper && tagMapper.responseData) tagMapper.responseData(traceSpan, response.data);
-      }
-      if (!traceSpan.endTime) traceSpan.close();
-    });
-    doNotInstrumentFollowingHttpRequest();
+    let traceSpan;
+    try {
+      // identifier
+      const serviceName =
+        this.service.constructor.serviceIdentifier ||
+        this.service.constructor.__super__.serviceIdentifier;
+      const tagMapper = serviceMapper.get(serviceName);
+      const operationName = this.operation.toLowerCase();
+      const params = this.params;
+      traceSpan = serverlessSdk._createTraceSpan(`aws.sdk.${serviceName}.${operationName}`, {
+        tags: {
+          'aws.sdk.region': this.service.config.region,
+          'aws.sdk.signature_version': this.service.config.signatureVersion,
+          'aws.sdk.service': serviceName,
+          'aws.sdk.operation': operationName,
+        },
+        input: shouldMonitorRequestResponse ? safeStringify(params) : null,
+      });
+      if (tagMapper && tagMapper.params) tagMapper.params(traceSpan, params);
+      let wasCompleted = false;
+      this.on('complete', (response) => {
+        try {
+          if (wasCompleted) {
+            console.warn({
+              source: 'serverlessSdk',
+              message:
+                'Detected doubled handling for same AWS SDK request. ' +
+                'It may happen if for the same request both callback and promise resolution ' +
+                'is requested. Internally it creates two AWS SDK calls so such design should be avoided.\n',
+              code: 'AWS_SDK_DOUBLE_RESOLUTION',
+            });
+            return;
+          }
+          wasCompleted = true;
+          if (response.requestId) traceSpan.tags.set('aws.sdk.request_id', response.requestId);
+          if (response.error) {
+            // Fallback to error.name, as there are cases when AWS SDK returns error with no message:
+            // https://github.com/aws/aws-sdk-js/issues/4330
+            traceSpan.tags.set('aws.sdk.error', response.error.message || response.error.name);
+          } else {
+            if (shouldMonitorRequestResponse) traceSpan.output = safeStringify(response.data);
+            if (tagMapper && tagMapper.responseData) {
+              tagMapper.responseData(traceSpan, response.data);
+            }
+          }
+          if (!traceSpan.endTime) traceSpan.close();
+        } catch (error) {
+          serverlessSdk._reportError(error);
+        }
+      });
+      doNotInstrumentFollowingHttpRequest();
+    } catch (error) {
+      serverlessSdk._reportError(error);
+      return originalRunTo.call(this, state, done);
+    }
     try {
       return originalRunTo.call(this, state, done);
     } finally {
@@ -94,4 +106,4 @@ module.exports.uninstall = (client) => {
   if (uninstall) uninstall();
 };
 
-const serverlessSdk = global.serverlessSdk || require('../');
+const serverlessSdk = require('../');
diff --git a/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v3-client.js b/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v3-client.js
index 891d9aca1..e4797bbed 100644
--- a/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v3-client.js
+++ b/node/packages/aws-lambda-sdk/instrumentation/aws-sdk-v3-client.js
@@ -27,23 +27,28 @@ module.exports.install = (client) => {
     applyToStack: (stack) => {
       // Middleware added to mark start and end of an complete API call.
       const awsRequestMiddleware = (next, context) => async (args) => {
-        const operationName = context.commandName.slice(0, -'Command'.length).toLowerCase();
-        const tagMapper = serviceMapper.get(serviceName);
-        const traceSpan = serverlessSdk._createTraceSpan(
-          `aws.sdk.${serviceName}.${operationName}`,
-          {
+        let deferredRegion;
+        let traceSpan;
+        let tagMapper;
+        try {
+          const operationName = context.commandName.slice(0, -'Command'.length).toLowerCase();
+          tagMapper = serviceMapper.get(serviceName);
+          traceSpan = serverlessSdk._createTraceSpan(`aws.sdk.${serviceName}.${operationName}`, {
             tags: {
               'aws.sdk.service': serviceName,
               'aws.sdk.operation': operationName,
               'aws.sdk.signature_version': 'v4',
             },
             input: shouldMonitorRequestResponse ? safeStringify(args.input) : null,
-          }
-        );
-        if (tagMapper && tagMapper.params) tagMapper.params(traceSpan, args.input);
-        const deferredRegion = client.config
-          .region()
-          .then((region) => traceSpan.tags.set('aws.sdk.region', region));
+          });
+          if (tagMapper && tagMapper.params) tagMapper.params(traceSpan, args.input);
+          deferredRegion = client.config
+            .region()
+            .then((region) => traceSpan.tags.set('aws.sdk.region', region));
+        } catch (error) {
+          serverlessSdk._reportSdkError(error);
+          return next(args);
+        }
 
         const nextDeferred = (() => {
           try {
@@ -76,7 +81,11 @@ module.exports.install = (client) => {
           traceSpan.tags.set('aws.sdk.request_id', response.output.$metadata.requestId);
           if (shouldMonitorRequestResponse) traceSpan.output = safeStringify(response.output);
           if (tagMapper && tagMapper.responseData) {
-            tagMapper.responseData(traceSpan, response.output);
+            try {
+              tagMapper.responseData(traceSpan, response.output);
+            } catch (sdkError) {
+              serverlessSdk._reportSdkError(sdkError);
+            }
           }
           if (!traceSpan.endTime) traceSpan.close();
           return response;
@@ -108,4 +117,4 @@ module.exports.uninstall = (client) => {
   if (uninstall) uninstall();
 };
 
-const serverlessSdk = global.serverlessSdk || require('../');
+const serverlessSdk = require('../');
diff --git a/node/packages/aws-lambda-sdk/internal-extension/wrapper.js b/node/packages/aws-lambda-sdk/internal-extension/wrapper.js
index 5d4daacf9..f1e6b71d2 100644
--- a/node/packages/aws-lambda-sdk/internal-extension/wrapper.js
+++ b/node/packages/aws-lambda-sdk/internal-extension/wrapper.js
@@ -13,7 +13,7 @@ delete EvalError.$serverlessHandlerModuleInstruction;
 const path = require('path');
 
 // 1. Initialize SDK instrumentation
-(() => {
+const serverlessSdk = (() => {
   try {
     require.resolve('@serverless/aws-lambda-sdk');
   } catch {
@@ -22,7 +22,12 @@ const path = require('path');
 
   // eslint-disable-next-line import/no-unresolved
   return require('@serverless/aws-lambda-sdk');
-})()._initialize();
+})();
+try {
+  serverlessSdk._initialize();
+} catch (error) {
+  serverlessSdk._reportSdkError(error);
+}
 const instrument = require('../instrument');
 
 // 2. Initialize original handler
@@ -84,12 +89,7 @@ const resolveHandlerObject = (resolvedHandlerModule) => {
   try {
     return { handler: instrument(handlerFunction) };
   } catch (error) {
-    process._rawDebug(
-      'Fatal Serverless SDK Error: ' +
-        'Please report at https://github.com/serverless/console/issues: ' +
-        'Async handler setup failed: ',
-      error && (error.stack || error)
-    );
+    serverlessSdk._reportSdkError(error);
     return { handler: handlerFunction };
   }
 };
diff --git a/node/packages/aws-lambda-sdk/lib/instrumentation/aws-sdk/index.js b/node/packages/aws-lambda-sdk/lib/instrumentation/aws-sdk/index.js
index 8af70d721..8faa59469 100644
--- a/node/packages/aws-lambda-sdk/lib/instrumentation/aws-sdk/index.js
+++ b/node/packages/aws-lambda-sdk/lib/instrumentation/aws-sdk/index.js
@@ -16,7 +16,11 @@ module.exports.install = () => {
     const originalSend = Client.prototype.send;
     const uninstallers = new Set();
     Client.prototype.send = function send(command, optionsOrCb, cb) {
-      uninstallers.add(instrumentV3Client(this));
+      try {
+        uninstallers.add(instrumentV3Client(this));
+      } catch (error) {
+        serverlessSdk._reportSdkError(error);
+      }
       return originalSend.call(this, command, optionsOrCb, cb);
     };
     const uninstall = () => {
@@ -34,3 +38,5 @@ module.exports.uninstall = () => {
   cjsHook.unregister('/aws-sdk/lib/core.js');
   cjsHook.unregister('/@aws-sdk/smithy-client/dist-cjs/client.js');
 };
+
+const serverlessSdk = require('../../../');
diff --git a/node/packages/aws-lambda-sdk/package.json b/node/packages/aws-lambda-sdk/package.json
index 362b2d12a..8b5022e3c 100644
--- a/node/packages/aws-lambda-sdk/package.json
+++ b/node/packages/aws-lambda-sdk/package.json
@@ -4,7 +4,7 @@
   "version": "0.14.2",
   "author": "Serverless, Inc.",
   "dependencies": {
-    "@serverless/sdk": "^0.4.1",
+    "@serverless/sdk": "^0.4.3",
     "@serverless/sdk-schema": "^0.14.5",
     "d": "^1.0.1",
     "ext": "^1.7.0",


Test patch:
diff --git a/node/packages/aws-lambda-sdk/test/unit/internal-extension/index.test.js b/node/packages/aws-lambda-sdk/test/unit/internal-extension/index.test.js
index 4d0f00dbd..3e6783cbb 100644
--- a/node/packages/aws-lambda-sdk/test/unit/internal-extension/index.test.js
+++ b/node/packages/aws-lambda-sdk/test/unit/internal-extension/index.test.js
@@ -55,7 +55,6 @@ const handleInvocation = async (handlerModuleName, options = {}) => {
     const serverlessSdk = require('../../../');
     const { TracePayload } = require('@serverless/sdk-schema/dist/trace');
     const { RequestResponse } = require('@serverless/sdk-schema/dist/request_response');
-    delete global.serverlessSdk;
     return {
       result,
       error,
@@ -136,6 +135,8 @@ describe('internal-extension/index.test.js', () => {
     process.env.LAMBDA_TASK_ROOT = path.resolve(fixturesDirname, 'lambdas');
     process.env.LAMBDA_RUNTIME_DIR = path.resolve(fixturesDirname, 'runtime');
     process.env.SLS_ORG_ID = 'dummy';
+    process.env.SLS_UNIT_TEST_RUN = '1';
+    process.env.SLS_CRASH_ON_SDK_ERROR = '1';
   });
   afterEach(() => {
     delete process.env._HANDLER;
diff --git a/node/test/lib/get-process-function.js b/node/test/lib/get-process-function.js
index db8d61cb9..8769cb70d 100644
--- a/node/test/lib/get-process-function.js
+++ b/node/test/lib/get-process-function.js
@@ -31,6 +31,7 @@ module.exports = async (basename, coreConfig, options) => {
         Variables: {
           SLS_ORG_ID: process.env.SLS_ORG_ID,
           SLS_SDK_DEBUG: '1',
+          SLS_CRASH_ON_SDK_ERROR: '1',
         },
       },
       Timeout: 15,