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

Unhandled "StopProfiling: Stopping Sentry profile did not return a profile" errors #196

Closed tomgrossman closed 1 year ago

tomgrossman commented 1 year ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

SDK Version

@sentry/* - 7.66.0, @sentry/profiling-node - 1.2.0

Link to Sentry event

https://guardz.sentry.io/issues/4452503110/events/9e36fc80170045e191ab84ebfb12db22/?project=6674202

What environment is your node script running in?

TS - Node.js, Nest.js, Express (all latest)

How is your code deployed and bundled?

dockerfile, installing and building

Steps to Reproduce

For some reason it doesn't reproduce locally, only in prod.

init:

import * as Sentry from '@sentry/node';
import * as Tracing from '@sentry/tracing';
import { RewriteFrames } from '@sentry/integrations';
import { ProfilingIntegration } from '@sentry/profiling-node';

Sentry.init({
      dsn: xxx,
      environment:'production',
      integrations: [
        new RewriteFrames({
          root: '/app',
        }),
        new Sentry.Integrations.Http({ tracing: true }),
        new Tracing.Integrations.Express({
          app: httpAdapter.getInstance(),
        }),
        new ProfilingIntegration(),
      ],
      tracesSampler: (samplingContext) => {
        if (samplingContext.request?.url?.includes('/health')) {
          return 0;
        }

        return 0.2;
      },
      profilesSampleRate: 1.0,
    });

worker (short version):

import * as Sentry from '@sentry/node';
import { OnWorkerEvent } from '@nestjs/bullmq';
import { Job } from 'bullmq';

export default class EventListenerWorker{

  private transaction: Sentry.Transaction;

  async process(job: Job): Promise<unknown> {
    return { success: true}
  }

  @OnWorkerEvent('active')
  onActive(job: Job) {
    this.transaction = Sentry.startTransaction({ op: 'worker', name: job.queueName, sampled: true });
  }

  @OnWorkerEvent('completed')
  async onCompleted() {
    this.transaction.finish();
  }

  @OnWorkerEvent('failed')
  async onFailed() {
    this.transaction.finish();
  }
}

Expected Result

end the transaction successfully.

Actual Result

We are getting lots of unhandled errors:

Error: StopProfiling: Stopping Sentry profile did not return a profile.                                                                                                                                                                                                    │
     at Object.stopProfiling (/app/node_modules/@sentry/profiling-node/lib/index.js:17507:39)                                                                                                                                                                               │
     at stopTransactionProfile (/app/node_modules/@sentry/profiling-node/lib/index.js:17866:39)                                                                                                                                                                             │
     at Timeout._onTimeout (/app/node_modules/@sentry/profiling-node/lib/index.js:17980:29)                                                                                                                                                                                 │
     at listOnTimeout (node:internal/timers:569:17)                                                                                                                                                                                                                         │
     at process.processTimers (node:internal/timers:512:7) {                                                                                                                                                                                                                │
   code: 'NAPI_ERROR'                                                                                                                                                                                                                                                       │
 }
JonasBa commented 1 year ago

Thanks for opening this @tomgrossman, does this still happen if you downgrade to 1.1.x?

tomgrossman commented 1 year ago

Thanks for opening this @tomgrossman, does this still happen if you downgrade to 1.1.x?

@JonasBa I didn't check 1.1.x because it happens only in prod, no idea why locally no, so I can't really do lots of tests, but with 0.3.0 it doesn't happen.

JonasBa commented 1 year ago

I'll investigate this asap, sorry about that.

JonasBa commented 1 year ago

@tomgrossman can onComplete and onFailed be called for the same transaction in your case?

I suspect this might be crashing if you stop the profile multiple times (we do have test and guard for this, but it might be escaping it somehow)

tomgrossman commented 1 year ago

@tomgrossman can onComplete and onFailed be called for the same transaction in your case?

I suspect this might be crashing if you stop the profile multiple times (we do have test and guard for this, but it might be escaping it somehow)

@JonasBa I checked that and I didn't see it happens in both, so I guess not, but I don't really know. What I do know that it didn't happen before and now all the time. Also if it fails only on the profile, not on the transaction, even if the transaction is already closed, the issue is with the profile, it doesn't need to throw an exception if it's already closed, like the transaction.

JonasBa commented 1 year ago

@tomgrossman yeah, we would need to know more about the runtime and possibly if the job.queueName is some invalid value here. I'm going to try and setup a few tests to see if I can replicate the crash, else I will remove the throw clause as it was added recently

JonasBa commented 1 year ago

@tomgrossman Please upgrade the SDK to 1.2.1 - I've removed the throw and changed the code so that nulptr profiles are gracefully handled. Sorry about that 🙏🏼

tomgrossman commented 1 year ago

@JonasBa thanks! looks good now