getsentry / profiling-node

The code for this repo now lives in https://github.com/getsentry/sentry-javascript/tree/develop/packages/profiling-node
MIT License
29 stars 10 forks source link

@sentry/profiling-node crashes vitest #228

Closed capaj closed 10 months ago

capaj commented 10 months ago

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

7.92.0

Framework Version

vitest any

Link to Sentry event

No response

SDK Setup

import * as SentryApi from '@sentry/node';
// Edit an assertion and save to see HMR in action
import { ProfilingIntegration } from '@sentry/profiling-node';

SentryApi.init({
  integrations: [new ProfilingIntegration()],
});

Steps to Reproduce

I tried to setup a minimal repro here: https://stackblitz.com/edit/vitest-dev-vitest-1gd3e9?file=src%2Fsentry.ts

but I failed, because profiling integration does not run on stackblitz. So what you need to do:

  1. download from stackblitz
  2. extract
  3. pnpm i
  4. pnpm test
  5. hit enter to restart the tests for few seconds(10 times probably+-)

Expected Result

Vitest does not crash

Actual Result

vitest crashes after a few runs Screencast from 5.1.2024 17:53:35.webm

AbhiPrasad commented 10 months ago

I'm going to transfer this to the profiling node repo, thanks for opening an issue! cc @JonasBa

JonasBa commented 10 months ago

@capaj do I understand correctly when you say that it crashes after some random number of test restarts? And do you mind me asking what version of node were you seeing the crash on?

capaj commented 10 months ago

Yes it crashes after roughly 10 runs

I have node 20 but it reproduces on 18 too

JonasBa commented 10 months ago

Yup, I managed to reproduce. I dont know for sure, but I suspect this might be related to how vitest spawns threads to run tests (I haven't looked into how vitest works yet) and we don't clean up the napi instance data correctly.

JonasBa commented 10 months ago

@capaj mind upgrading to 1.3.3 and testing this? I just tested the code you provided and failed to reproduce the issue.

daniluk4000 commented 10 months ago

I still have the same error. After I have disabled profiling (on dev), errors stopped from happening.

Errors were also followed by ECONNRESET and ECONNREFUSED in node console.

Didn't notice same error on production.