Merge branch 'develop' into abhi-sentry-origin-logs

Author: AbhiPrasad

CHANGELOG.md

@@ -10,7 +10,7 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
-Work in this release was contributed by @Page-. Thank you for your contribution!
+Work in this release was contributed by @Page- and @Fryuni. Thank you for your contributions!
 
 ## 9.11.0
 

packages/astro/src/server/middleware.ts

@@ -184,12 +184,18 @@ async function instrumentRequest(
 
               const newResponseStream = new ReadableStream({
                 start: async controller => {
-                  for await (const chunk of originalBody) {
-                    const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true });
-                    const modifiedHtml = addMetaTagToHead(html);
-                    controller.enqueue(new TextEncoder().encode(modifiedHtml));
+                  try {
+                    for await (const chunk of originalBody) {
+                      const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true });
+                      const modifiedHtml = addMetaTagToHead(html);
+                      controller.enqueue(new TextEncoder().encode(modifiedHtml));
+                    }
+                  } catch (e) {
+                    sendErrorToSentry(e);
+                    controller.error(e);
+                  } finally {
+                    controller.close();
                   }
-                  controller.close();
                 },
               });
 

packages/astro/test/server/middleware.test.ts

@@ -149,6 +149,49 @@ describe('sentryMiddleware', () => {
     });
   });
 
+  it('throws and sends an error to sentry if response streaming throws', async () => {
+    const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException');
+
+    const middleware = handleRequest();
+    const ctx = {
+      request: {
+        method: 'GET',
+        url: '/users',
+        headers: new Headers(),
+      },
+      url: new URL('https://myDomain.io/users/'),
+      params: {},
+    };
+
+    const error = new Error('Something went wrong');
+
+    const faultyStream = new ReadableStream({
+      pull: controller => {
+        controller.error(error);
+        controller.close();
+      },
+    });
+
+    const next = vi.fn(() =>
+      Promise.resolve(
+        new Response(faultyStream, {
+          headers: new Headers({ 'content-type': 'text/html' }),
+        }),
+      ),
+    );
+
+    // @ts-expect-error, a partial ctx object is fine here
+    const resultFromNext = await middleware(ctx, next);
+
+    expect(resultFromNext?.headers.get('content-type')).toEqual('text/html');
+
+    await expect(() => resultFromNext!.text()).rejects.toThrowError();
+
+    expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
+      mechanism: { handled: false, type: 'astro', data: { function: 'astroMiddleware' } },
+    });
+  });
+
   describe('track client IP address', () => {
     it('attaches client IP if `trackClientIp=true` when handling dynamic page requests', async () => {
       const middleware = handleRequest({ trackClientIp: true });

packages/cloudflare/README.md

@@ -22,16 +22,21 @@ To get started, first install the `@sentry/cloudflare` package:
 npm install @sentry/cloudflare
 ```
 
-Then set either the `nodejs_compat` or `nodejs_als` compatibility flags in your `wrangler.toml`. This is because the SDK
-needs access to the `AsyncLocalStorage` API to work correctly.
-
-```toml
-compatibility_flags = ["nodejs_compat"]
-# compatibility_flags = ["nodejs_als"]
+Then either set the `nodejs_als` or `nodejs_compat` compatibility flags in your `wrangler.jsonc`/`wrangler.toml` config. This is because the SDK needs access to the `AsyncLocalStorage` API to work correctly.
+
+```jsonc {tabTitle:JSON} {filename:wrangler.jsonc}
+{
+  "compatibility_flags": [
+    "nodejs_als"
+    // "nodejs_compat"
+  ]
+}
 ```
 
-Then you can either setup up the SDK for [Cloudflare Pages](#setup-cloudflare-pages) or
-[Cloudflare Workers](#setup-cloudflare-workers).
+```toml {tabTitle:Toml} {filename:wrangler.toml}
+compatibility_flags = ["nodejs_als"]
+# compatibility_flags = ["nodejs_compat"]
+```
 
 ## Setup (Cloudflare Pages)
 

packages/core/src/client.ts

@@ -670,6 +670,13 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
    */
   public on(hook: 'afterCaptureLog', callback: (log: Log) => void): () => void;
 
+  /**
+   * A hook that is called when the client is flushing logs
+   *
+   * @returns {() => void} A function that, when executed, removes the registered callback.
+   */
+  public on(hook: 'flushLogs', callback: () => void): () => void;
+
   /**
    * Register a hook on this client.
    */
@@ -827,6 +834,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
    */
   public emit(hook: 'afterCaptureLog', log: Log): void;
 
+  /**
+   * Emit a hook event for client flush logs
+   */
+  public emit(hook: 'flushLogs'): void;
+
   /**
    * Emit a hook that was previously registered via `on()`.
    */

packages/core/src/logs/exports.ts

@@ -163,6 +163,8 @@ export function _INTERNAL_flushLogsBuffer(client: Client, maybeLogBuffer?: Array
   // Clear the log buffer after envelopes have been constructed.
   logBuffer.length = 0;
 
+  client.emit('flushLogs');
+
   // sendEnvelope should not throw
   // eslint-disable-next-line @typescript-eslint/no-floating-promises
   client.sendEnvelope(envelope);

packages/core/src/server-runtime-client.ts

@@ -25,6 +25,9 @@ import { resolvedSyncPromise } from './utils-hoist/syncpromise';
 import { _INTERNAL_flushLogsBuffer } from './logs/exports';
 import { isPrimitive } from './utils-hoist';
 
+// TODO: Make this configurable
+const DEFAULT_LOG_FLUSH_INTERVAL = 5000;
+
 export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportOptions> {
   platform?: string;
   runtime?: { name: string; version?: string };
@@ -37,6 +40,7 @@ export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportO
 export class ServerRuntimeClient<
   O extends ClientOptions & ServerRuntimeClientOptions = ServerRuntimeClientOptions,
 > extends Client<O> {
+  private _logFlushIdleTimeout: ReturnType<typeof setTimeout> | undefined;
   private _logWeight: number;
 
   /**
@@ -54,9 +58,9 @@ export class ServerRuntimeClient<
     if (this._options._experiments?.enableLogs) {
       // eslint-disable-next-line @typescript-eslint/no-this-alias
       const client = this;
-      client.on('flush', () => {
-        _INTERNAL_flushLogsBuffer(client);
+      client.on('flushLogs', () => {
         client._logWeight = 0;
+        clearTimeout(client._logFlushIdleTimeout);
       });
 
       client.on('afterCaptureLog', log => {
@@ -67,7 +71,11 @@ export class ServerRuntimeClient<
         // the payload gets too big.
         if (client._logWeight >= 800_000) {
           _INTERNAL_flushLogsBuffer(client);
-          client._logWeight = 0;
+        } else {
+          // start an idle timeout to flush the logs buffer if no logs are captured for a while
+          client._logFlushIdleTimeout = setTimeout(() => {
+            _INTERNAL_flushLogsBuffer(client);
+          }, DEFAULT_LOG_FLUSH_INTERVAL);
         }
       });
     }

packages/core/test/lib/server-runtime-client.test.ts

@@ -4,7 +4,7 @@ import { describe, expect, it, test, vi } from 'vitest';
 import { Scope, createTransport } from '../../src';
 import type { ServerRuntimeClientOptions } from '../../src/server-runtime-client';
 import { ServerRuntimeClient } from '../../src/server-runtime-client';
-import { _INTERNAL_captureLog } from '../../src/logs/exports';
+import { _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer } from '../../src/logs/exports';
 
 const PUBLIC_DSN = 'https://username@domain/123';
 
@@ -256,8 +256,8 @@ describe('ServerRuntimeClient', () => {
       _INTERNAL_captureLog({ message: 'test1', level: 'info' }, client);
       _INTERNAL_captureLog({ message: 'test2', level: 'info' }, client);
 
-      // Trigger flush event
-      client.emit('flush');
+      // Trigger flush directly
+      _INTERNAL_flushLogsBuffer(client);
 
       expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1);
       expect(client['_logWeight']).toBe(0); // Weight should be reset after flush

packages/node/src/sdk/client.ts

@@ -4,7 +4,7 @@ import { trace } from '@opentelemetry/api';
 import { registerInstrumentations } from '@opentelemetry/instrumentation';
 import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
 import type { DynamicSamplingContext, Scope, ServerRuntimeClientOptions, TraceContext } from '@sentry/core';
-import { SDK_VERSION, ServerRuntimeClient, applySdkMetadata, logger } from '@sentry/core';
+import { _INTERNAL_flushLogsBuffer, SDK_VERSION, ServerRuntimeClient, applySdkMetadata, logger } from '@sentry/core';
 import { getTraceContextForScope } from '@sentry/opentelemetry';
 import { isMainThread, threadId } from 'worker_threads';
 import { DEBUG_BUILD } from '../debug-build';
@@ -18,6 +18,7 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
   private _tracer: Tracer | undefined;
   private _clientReportInterval: NodeJS.Timeout | undefined;
   private _clientReportOnExitFlushListener: (() => void) | undefined;
+  private _logOnExitFlushListener: (() => void) | undefined;
 
   public constructor(options: NodeClientOptions) {
     const clientOptions: ServerRuntimeClientOptions = {
@@ -40,6 +41,14 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
     );
 
     super(clientOptions);
+
+    if (this.getOptions()._experiments?.enableLogs) {
+      this._logOnExitFlushListener = () => {
+        _INTERNAL_flushLogsBuffer(this);
+      };
+
+      process.on('beforeExit', this._logOnExitFlushListener);
+    }
   }
 
   /** Get the OTEL tracer. */
@@ -84,6 +93,10 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
       process.off('beforeExit', this._clientReportOnExitFlushListener);
     }
 
+    if (this._logOnExitFlushListener) {
+      process.off('beforeExit', this._logOnExitFlushListener);
+    }
+
     return super.close(timeout);
   }