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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions