Configuration is not skipped when DSN is missing

[petr7555]

The log at https://github.com/immobiliare/fastify-sentry/blob/next/index.js#L70 implies that Sentry configuration should be skipped when DSN is not provided. But it is clearly initialized a few lines above.

My Context

I get DSN from an environment variable, which is available only in prod. During local development the DSN is empty. Because fastify-sentry still initializes the Sentry, when an exception occurs, a long Sentry Logger [error]: SentryError: SDK not enabled, will not capture event. error is logged to the console, which is unwanted for me as it clutters the console.

I am aware I could wrap the plugin registration into if (env !== 'development'), but then the fastify.Sentry is undefined, causing calls like fastify.Sentry.addBreadcrumb to fail and the TS does not warn about it, because the plugin provides the following type:

declare module 'fastify' {
    interface FastifyInstance {
        Sentry: typeof Sentry;
    }
}

I see two solutions:

1. Overwrite the 'fastify' module type with

declare module 'fastify' {
    interface FastifyInstance {
        Sentry?: typeof Sentry;
    }
}

and then write fastify.Sentry?.addBreadcrumb every time (with ?).

2. Do not call Sentry.init() in the plugin unless DSN is provided.

I am not aware on how to do the first without changing the code of this plugin, otherwise I would have done it.
The second solution also seems reasonable to me and I think it is a bug that it is initialized even though the log says it won't be. Therefore I am opening this issue.

[simonecorsi]

Hi @petr7555, thanks for your detailed report! 🙏

At the moment I haven't had your problem (yet 😄) but I can see your point and it could be useful how we handle initialization!

If we're going to change this, I would prefer something like your second solution, which is more solid in the long term!

Anyway, we're going to have a proper look at this tomorrow morning to sort this out, I'll keep you updated!

[dnlup]

The log at https://github.com/immobiliare/fastify-sentry/blob/next/index.js#L70 implies that Sentry configuration should be skipped when DSN is not provided. But it is clearly initialized a few lines above.

Hi @petr7555 , you're right about the message log being misleading. We missed that, we'll definitely change it now.

As for your context, it's the same context we have too. The Sentry SDK is pretty smart and doesn't send events if a DSN is not passed when calling init, so I definitely agree with you that wrapping the plugin registration in an if isn't right and causes issues with types definitions. The log message you are talking about is printed only if you pass the debug option (or a env var maybe?) to the plugin and hence the SDK itself. Is it required to have the SDK running in debug mode?

[petr7555]

Hi @dnlup. You are right, I was passing debug: True in the Sentry options. I've removed it and now the Sentry is silent when the DNS is missing.

[dnlup]

Happy that it solved your issue. We'll fix the log message though, thank you for reporting it 😉

[petr7555]

Thank you for your quick response and for pointing out the debug option.