Skip to content

node: Healthy process sessions are never sent because the beforeExit check is inverted #23699

Description

@suhailopensource

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

10.70.0

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

processSessionIntegration is an unconditional default integration (packages/node/src/sdk/index.ts:73, and packages/bun/src/sdk.ts:66 for @sentry/bun), so no opt-in is needed to hit this. Any plain Node process reproduces it:

Sentry.init({ dsn: __YOUR_DSN__, release: '1.0.0' });
// ...do some work, capture no errors...
// process exits normally -> expect one session envelope with status "exited"

A unit test in this repo reproduces it without a network. Save as packages/node/test/integrations/processSession.test.ts and run cd packages/node && yarn vitest run test/integrations/processSession.test.ts:

import { getIsolationScope, setCurrentClient } from '@sentry/core';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { processSessionIntegration } from '../../src/integrations/processSession';
import { NodeClient } from '../../src/sdk/client';
import { getDefaultNodeClientOptions } from '../helpers/getDefaultNodeClientOptions';

describe('processSessionIntegration', () => {
  let client: NodeClient;
  let sendSession: ReturnType<typeof vi.spyOn>;
  let beforeExitHandler: () => void;

  beforeEach(() => {
    getIsolationScope().setSession(undefined);
    client = new NodeClient(getDefaultNodeClientOptions({ release: '1.0.0' }));
    setCurrentClient(client);
    client.init();
    sendSession = vi.spyOn(client, 'sendSession').mockImplementation(() => undefined);

    const onSpy = vi.spyOn(process, 'on').mockImplementation(((event: string, cb: () => void) => {
      if (event === 'beforeExit') beforeExitHandler = cb;
      return process;
    }) as never);
    processSessionIntegration().setupOnce!();
    onSpy.mockRestore();
  });

  it('sends one session with status "exited" on a healthy exit', () => {
    beforeExitHandler();
    expect(sendSession).toHaveBeenCalledTimes(1);
    expect(sendSession).toHaveBeenCalledWith(expect.objectContaining({ status: 'exited' }));
  });

  it('does not send a second update for an already-crashed session', () => {
    getIsolationScope().getSession()!.status = 'crashed';
    sendSession.mockClear();
    beforeExitHandler();
    expect(sendSession).toHaveBeenCalledTimes(0);
  });
});

Steps to Reproduce

  1. Run any Node process with Sentry.init({ dsn, release }) that does some work and exits cleanly, capturing no errors.
  2. Look for a session envelope in the SDK output — or add the unit test above and run it.
  3. Or just read packages/node/src/integrations/processSession.ts; the whole file is 31 lines:
// Only call endSession, if the Session exists on Scope and SessionStatus is not a
// Terminal Status i.e. Exited or Crashed because
// "When a session is moved away from ok it must not be updated anymore."
// Ref: https://develop.sentry.dev/sdk/sessions/
if (session?.status !== 'ok') {
  endSession();
}

The guard the comment describes is "session exists and status is not exited/crashed". What is written is its inverse for the healthy case.

Expected Result

Per the comment's own description and https://develop.sentry.dev/sdk/sessions/:

  • healthy exit → exactly one session envelope, status exited
  • already-crashed session → no further update for that sid, since it is terminal

Both tests above pass.

Actual Result

  • healthy exit → zero session envelopes; nothing is ever sent for the process
  • already-crashed session → one further envelope, a post-terminal update for the same sid

Both tests above fail, against develop @ 3016b1f:

 × processSessionIntegration > sends one session with status "exited" on a healthy exit
   → expected "sendSession" to be called 1 times, but got 0 times
 × processSessionIntegration > does not send a second update for an already-crashed session
   → expected "sendSession" to be called +0 times, but got 1 times

 Tests  2 failed (2)

Inspecting the spy calls directly shows the healthy run records no sendSession call at all, and the crashed run records one with status: "crashed".

Additional Context

Why the healthy case sends nothing at all

startSession() (packages/core/src/exports.ts) only creates the session and puts it on the isolation scope — it does not send it. The only route to a type: 'session' envelope is:

endSession() -> closeSession() -> _sendSessionUpdate() -> client.captureSession() -> sendSession()

The browser SDK compensates with an explicit deferred captureSession() (packages/browser/src/integrations/browsersession.ts), but Node has no initial send — and a repo-wide grep finds exactly one non-export endSession() call site across packages/node/src and packages/bun/src: this handler, at processSession.ts:26. So when the guard skips it, nothing is sent for the entire process run.

This is not covered by the request-session aggregates. Those come from packages/core/src/integrations/http/record-request-session.ts via client.sendSession({ aggregates }) and only apply to an HTTP server, so scripts, CLIs, workers and serverless invocations are left with nothing.

Provenance — the guard was inverted at some point

PR #3423 ("feat(node): Application mode sessions") introduced this handler with the same comment that is still present today, and the condition was:

const terminalStates = [SessionStatus.Exited, SessionStatus.Crashed];
// Only call endSession, if the Session exists on Scope and SessionStatus is not a
// Terminal Status i.e. Exited or Crashed because
if (session && !terminalStates.includes(session.status)) hub.endSession();

The current session?.status !== 'ok' is the logical inverse of that for the healthy case, and it additionally fires when session is undefined — which the comment's "if the Session exists on Scope" explicitly excludes.

That PR also added a manual test asserting a healthy session is sent on beforeExit (packages/node/test/manual/release-health/single-session/healthy-session.js). No release-health directory or healthy-session* file exists in the repo any more, so nothing guards this behaviour today.

Nothing asserts the current behaviour either: there is no processSession test anywhere in the repo, and with the guard restored the full @sentry/node unit suite still passes (32 files, 366 tests).

Suggested fix

One line, restoring what the comment describes:

if (session && session.status !== 'exited' && session.status !== 'crashed') {

Verified: with that change the unit test above passes in both directions and the node suite stays green. Happy to open a PR with the test.

Related, but distinct

Affected versions

Present in 10.70.0. In that release the file lived at packages/node-core/src/integrations/processSession.ts and is byte-identical to the current packages/node/src/integrations/processSession.ts (packages/node-core has since been removed).

Priority

No response

Metadata

Metadata

Assignees

Projects

Status
Waiting for: Product Owner

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions