feat(firebase): add error handling and rename hooks to align with otel standards

JPeer264 · JPeer264 · commit cd4cd56f3ab4 · 2025-10-21T11:24:42.000+02:00
diff --git a/dev-packages/e2e-tests/test-applications/node-firebase/functions/src/index.ts b/dev-packages/e2e-tests/test-applications/node-firebase/functions/src/index.ts
@@ -18,6 +18,13 @@ export const helloWorld = onRequest(async (request, response) => {
   response.send('Hello from Firebase!');
 });
 
+export const unhandeledError = onRequest(async (request, response) => {
+  console.log('unhandeledError');
+  throw new Error('There is an error!');
+
+  response.send('Hello from Firebase!');
+});
+
 export const onCallSomething = onRequest(async (request, response) => {
   const data = {
     name: request.body?.name || 'Sample Document',
diff --git a/dev-packages/e2e-tests/test-applications/node-firebase/tests/functions.test.ts b/dev-packages/e2e-tests/test-applications/node-firebase/tests/functions.test.ts
@@ -1,5 +1,5 @@
 import { expect, test } from '@playwright/test';
-import { waitForTransaction } from '@sentry-internal/test-utils';
+import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
 
 test('should only call the function once without any extra calls', async () => {
   const serverTransactionPromise = waitForTransaction('node-firebase', span => {
@@ -15,6 +15,7 @@ test('should only call the function once without any extra calls', async () => {
     expect.objectContaining({
       trace: expect.objectContaining({
         data: {
+          'cloud.project_id': 'demo-functions',
           'faas.name': 'helloWorld',
           'faas.provider': 'firebase',
           'faas.trigger': 'http.request',
@@ -34,6 +35,35 @@ test('should only call the function once without any extra calls', async () => {
   );
 });
 
+test('should send failed transaction when the function fails', async () => {
+  const errorEventPromise = waitForError('node-firebase', () => true);
+  const serverTransactionPromise = waitForTransaction('node-firebase', span => {
+    return !!span.transaction;
+  });
+
+  await fetch(`http://localhost:5001/demo-functions/default/unhandeledError`);
+
+  const transactionEvent = await serverTransactionPromise;
+  const errorEvent = await errorEventPromise;
+
+  expect(transactionEvent.transaction).toEqual('firebase.function.http.request');
+  expect(transactionEvent.contexts?.trace?.trace_id).toEqual(errorEvent.contexts?.trace?.trace_id);
+  expect(errorEvent).toMatchObject({
+    exception: {
+      values: [
+        {
+          type: 'Error',
+          value: 'There is an error!',
+          mechanism: {
+            type: 'auto.firebase.otel.functions',
+            handled: false,
+          },
+        }
+      ],
+    },
+  });
+});
+
 test('should create a document and trigger onDocumentCreated and another with authContext', async () => {
   const serverTransactionPromise = waitForTransaction('node-firebase', span => {
     return span.transaction === 'firebase.function.http.request';
@@ -62,6 +92,7 @@ test('should create a document and trigger onDocumentCreated and another with au
   expect(transactionEvent.transaction).toEqual('firebase.function.http.request');
   expect(transactionEvent.contexts?.trace).toEqual({
     data: {
+      'cloud.project_id': 'demo-functions',
       'faas.name': 'onCallSomething',
       'faas.provider': 'firebase',
       'faas.trigger': 'http.request',
@@ -80,6 +111,7 @@ test('should create a document and trigger onDocumentCreated and another with au
   expect(transactionEvent.spans).toHaveLength(3);
   expect(transactionEventOnDocumentCreate.contexts?.trace).toEqual({
     data: {
+      'cloud.project_id': 'demo-functions',
       'faas.name': 'onDocumentCreate',
       'faas.provider': 'firebase',
       'faas.trigger': 'firestore.document.created',
@@ -98,6 +130,7 @@ test('should create a document and trigger onDocumentCreated and another with au
   expect(transactionEventOnDocumentCreate.spans).toHaveLength(2);
   expect(transactionEventOnDocumentWithAuthContextCreate.contexts?.trace).toEqual({
     data: {
+      'cloud.project_id': 'demo-functions',
       'faas.name': 'onDocumentCreateWithAuthContext',
       'faas.provider': 'firebase',
       'faas.trigger': 'firestore.document.created',
diff --git a/packages/node/src/integrations/tracing/firebase/firebase.ts b/packages/node/src/integrations/tracing/firebase/firebase.ts
@@ -1,5 +1,5 @@
 import type { IntegrationFn } from '@sentry/core';
-import { defineIntegration, SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core';
+import { captureException, defineIntegration, flush, SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core';
 import { addOriginToSpan, generateInstrumentOnce } from '@sentry/node-core';
 import { type FirebaseInstrumentationConfig, FirebaseInstrumentation } from './otel';
 
@@ -11,10 +11,23 @@ const config: FirebaseInstrumentationConfig = {
 
     span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'db.query');
   },
-  functionsSpanCreationHook: span => {
-    addOriginToSpan(span, 'auto.firebase.otel.functions');
+  functions: {
+    requestHook: span => {
+      addOriginToSpan(span, 'auto.firebase.otel.functions');
 
-    span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.request');
+      span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.request');
+    },
+    errorHook: async (_, error) => {
+      if (error) {
+        captureException(error, {
+          mechanism: {
+            type: 'auto.firebase.otel.functions',
+            handled: false,
+          }
+        });
+        await flush(2000);
+      }
+    },
   },
 };
 
diff --git a/packages/node/src/integrations/tracing/firebase/otel/patches/functions.ts b/packages/node/src/integrations/tracing/firebase/otel/patches/functions.ts
@@ -6,7 +6,6 @@ import {
   InstrumentationNodeModuleFile,
   isWrapped,
   safeExecuteInTheMiddle,
-  safeExecuteInTheMiddleAsync,
 } from '@opentelemetry/instrumentation';
 import type { SpanAttributes } from '@sentry/core';
 import type {
@@ -28,7 +27,7 @@ import type {
   onObjectMetadataUpdated,
 } from 'firebase-functions/storage';
 import type { FirebaseInstrumentation } from '../firebaseInstrumentation';
-import type { FirebaseInstrumentationConfig, FunctionsSpanCreationHook } from '../types';
+import type { FirebaseInstrumentationConfig, RequestHook, ResponseHook } from '../types';
 
 /**
  * Patches Firebase Functions v2 to add OpenTelemetry instrumentation
@@ -45,15 +44,30 @@ export function patchFunctions(
   unwrap: InstrumentationBase['_unwrap'],
   config: FirebaseInstrumentationConfig,
 ): InstrumentationNodeModuleDefinition {
-  const defaultFunctionsSpanCreationHook: FunctionsSpanCreationHook = () => {};
+  let requestHook: RequestHook = () => {};
+  let responseHook: ResponseHook = () => {};
+  const errorHook = config.functions?.errorHook;
+  const configRequestHook = config.functions?.requestHook;
+  const configResponseHook = config.functions?.responseHook;
 
-  let functionsSpanCreationHook: FunctionsSpanCreationHook = defaultFunctionsSpanCreationHook;
-  const configFunctionsSpanCreationHook = config.functionsSpanCreationHook;
-
-  if (typeof configFunctionsSpanCreationHook === 'function') {
-    functionsSpanCreationHook = (span: Span) => {
+  if (typeof configResponseHook === 'function') {
+    responseHook = (span: Span, err: unknown) => {
       safeExecuteInTheMiddle(
-        () => configFunctionsSpanCreationHook(span),
+        () => configResponseHook(span, err),
+        error => {
+          if (!error) {
+            return;
+          }
+          diag.error(error?.message);
+        },
+        true,
+      );
+    };
+  }
+  if (typeof configRequestHook === 'function') {
+    requestHook = (span: Span) => {
+      safeExecuteInTheMiddle(
+        () => configRequestHook(span),
         error => {
           if (!error) {
             return;
@@ -79,7 +93,14 @@ export function patchFunctions(
         name,
         functionsSupportedVersions,
         moduleExports =>
-          wrapCommonFunctions(moduleExports, wrap, unwrap, tracer, functionsSpanCreationHook, triggerType),
+          wrapCommonFunctions(
+            moduleExports,
+            wrap,
+            unwrap,
+            tracer,
+            { requestHook, responseHook, errorHook },
+            triggerType,
+          ),
         moduleExports => unwrapCommonFunctions(moduleExports, unwrap),
       ),
     );
@@ -123,17 +144,44 @@ type AvailableFirebaseFunctions = {
 
 type FirebaseFunctions = AvailableFirebaseFunctions[keyof AvailableFirebaseFunctions];
 
+/**
+ * Async function to execute patched function and being able to catch errors
+ * @param execute - function to be executed
+ * @param onFinish - callback to run when execute finishes
+ */
+export async function safeExecuteInTheMiddleAsync<T>(
+  execute: () => T,
+  onFinish: (e: Error | undefined, result: T | undefined) => Promise<void> | void,
+  preventThrowingError?: boolean,
+): Promise<T> {
+  let error: Error | undefined;
+  let result: T | undefined;
+  try {
+    result = await execute();
+  } catch (e) {
+    error = e as Error;
+  } finally {
+    await onFinish?.(error, result);
+    if (error && !preventThrowingError) {
+      // eslint-disable-next-line no-unsafe-finally
+      throw error;
+    }
+    // eslint-disable-next-line no-unsafe-finally
+    return result as T;
+  }
+}
+
 /**
  * Patches Cloud Functions for Firebase (v2) to add OpenTelemetry instrumentation
  *
  * @param tracer - Opentelemetry Tracer
- * @param functionsSpanCreationHook - Function to create a span for the function
+ * @param functionsConfig - Firebase instrumentation config
  * @param triggerType - Type of trigger
  * @returns A function that patches the function
  */
 export function patchV2Functions<T extends FirebaseFunctions = FirebaseFunctions>(
   tracer: Tracer,
-  functionsSpanCreationHook: FunctionsSpanCreationHook,
+  functionsConfig: FirebaseInstrumentationConfig['functions'],
   triggerType: string,
 ): (original: T) => (...args: OverloadedParameters<T>) => ReturnType<T> {
   return function v2FunctionsWrapper(original: T): (...args: OverloadedParameters<T>) => ReturnType<T> {
@@ -166,20 +214,35 @@ export function patchV2Functions<T extends FirebaseFunctions = FirebaseFunctions
         }
 
         span.setAttributes(attributes);
-        functionsSpanCreationHook(span);
-
-        return context.with(trace.setSpan(context.active(), span), () =>
-          safeExecuteInTheMiddleAsync(
-            () => handler.apply(this, handlerArgs),
-            err => {
-              if (err) {
-                span.recordException(err);
-              }
-
-              span.end();
-            },
-          ),
-        );
+        functionsConfig?.requestHook?.(span);
+
+        // Can be changed to safeExecuteInTheMiddleAsync once following is merged and released
+        // https://github.com/open-telemetry/opentelemetry-js/pull/6032
+        return context.with(trace.setSpan(context.active(), span), async () => {
+          let error: Error | undefined;
+          let result: T | undefined;
+
+          try {
+            result = await handler.apply(this, handlerArgs);
+          } catch (e) {
+            error = e as Error;
+          }
+
+          functionsConfig?.responseHook?.(span, error);
+
+          if (error) {
+            span.recordException(error);
+          }
+
+          span.end();
+
+          if (error) {
+            await functionsConfig?.errorHook?.(span, error);
+            throw error;
+          }
+
+          return result;
+        });
       };
 
       if (documentOrOptions) {
@@ -196,86 +259,58 @@ function wrapCommonFunctions(
   wrap: InstrumentationBase<FirebaseInstrumentationConfig>['_wrap'],
   unwrap: InstrumentationBase<FirebaseInstrumentationConfig>['_unwrap'],
   tracer: Tracer,
-  functionsSpanCreationHook: FunctionsSpanCreationHook,
+  functionsConfig: FirebaseInstrumentationConfig['functions'],
   triggerType: 'function' | 'firestore' | 'scheduler' | 'storage',
 ): AvailableFirebaseFunctions {
   unwrapCommonFunctions(moduleExports, unwrap);
 
   switch (triggerType) {
     case 'function':
-      wrap(moduleExports, 'onRequest', patchV2Functions(tracer, functionsSpanCreationHook, 'http.request'));
-      wrap(moduleExports, 'onCall', patchV2Functions(tracer, functionsSpanCreationHook, 'http.call'));
+      wrap(moduleExports, 'onRequest', patchV2Functions(tracer, functionsConfig, 'http.request'));
+      wrap(moduleExports, 'onCall', patchV2Functions(tracer, functionsConfig, 'http.call'));
       break;
 
     case 'firestore':
-      wrap(
-        moduleExports,
-        'onDocumentCreated',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.created'),
-      );
-      wrap(
-        moduleExports,
-        'onDocumentUpdated',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.updated'),
-      );
-      wrap(
-        moduleExports,
-        'onDocumentDeleted',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.deleted'),
-      );
-      wrap(
-        moduleExports,
-        'onDocumentWritten',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.written'),
-      );
+      wrap(moduleExports, 'onDocumentCreated', patchV2Functions(tracer, functionsConfig, 'firestore.document.created'));
+      wrap(moduleExports, 'onDocumentUpdated', patchV2Functions(tracer, functionsConfig, 'firestore.document.updated'));
+      wrap(moduleExports, 'onDocumentDeleted', patchV2Functions(tracer, functionsConfig, 'firestore.document.deleted'));
+      wrap(moduleExports, 'onDocumentWritten', patchV2Functions(tracer, functionsConfig, 'firestore.document.written'));
       wrap(
         moduleExports,
         'onDocumentCreatedWithAuthContext',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.created'),
+        patchV2Functions(tracer, functionsConfig, 'firestore.document.created'),
       );
       wrap(
         moduleExports,
         'onDocumentUpdatedWithAuthContext',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.updated'),
+        patchV2Functions(tracer, functionsConfig, 'firestore.document.updated'),
       );
 
       wrap(
         moduleExports,
         'onDocumentDeletedWithAuthContext',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.deleted'),
+        patchV2Functions(tracer, functionsConfig, 'firestore.document.deleted'),
       );
 
       wrap(
         moduleExports,
         'onDocumentWrittenWithAuthContext',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'firestore.document.written'),
+        patchV2Functions(tracer, functionsConfig, 'firestore.document.written'),
       );
       break;
 
     case 'scheduler':
-      wrap(moduleExports, 'onSchedule', patchV2Functions(tracer, functionsSpanCreationHook, 'scheduler.scheduled'));
+      wrap(moduleExports, 'onSchedule', patchV2Functions(tracer, functionsConfig, 'scheduler.scheduled'));
       break;
 
     case 'storage':
-      wrap(
-        moduleExports,
-        'onObjectFinalized',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'storage.object.finalized'),
-      );
-      wrap(
-        moduleExports,
-        'onObjectArchived',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'storage.object.archived'),
-      );
-      wrap(
-        moduleExports,
-        'onObjectDeleted',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'storage.object.deleted'),
-      );
+      wrap(moduleExports, 'onObjectFinalized', patchV2Functions(tracer, functionsConfig, 'storage.object.finalized'));
+      wrap(moduleExports, 'onObjectArchived', patchV2Functions(tracer, functionsConfig, 'storage.object.archived'));
+      wrap(moduleExports, 'onObjectDeleted', patchV2Functions(tracer, functionsConfig, 'storage.object.deleted'));
       wrap(
         moduleExports,
         'onObjectMetadataUpdated',
-        patchV2Functions(tracer, functionsSpanCreationHook, 'storage.object.metadataUpdated'),
+        patchV2Functions(tracer, functionsConfig, 'storage.object.metadataUpdated'),
       );
       break;
   }
diff --git a/packages/node/src/integrations/tracing/firebase/otel/types.ts b/packages/node/src/integrations/tracing/firebase/otel/types.ts
