immobiliare / fastify-sentry Goto Github PK
View Code? Open in Web Editor NEW๐ง Sentry errors handler for fastify that just works! Install, add your DSN and you're good to go!
License: MIT License
๐ง Sentry errors handler for fastify that just works! Install, add your DSN and you're good to go!
License: MIT License
Sentry is using some new api internally in its integrations, let's update ours to reflect that.
(node:15939) [FSTDEP012] FastifyDeprecation: Request#context property access is deprecated. Please use "Request#routeConfig" or "Request#routeSchema" instead for accessing Route settings. The "Request#context" will be removed in fastify@5
.
probably from line in utils.js:88
can I send PR?
We should change the default setting of Sentry integration to include the uncaughtException
handler.
Use tap
as a test runner.
I am not clear whether fastify-sentry
is needed if I am already using @sentry/opentelemetry
, and my goal is to just capture performance traces. As far as I understand, @opentelemetry/auto-instrumentations-node
already handles instrumentation for us.
It looks like some types are not resolved on the fastify.Sentry
instance as they would when using the Sentry
instance.
Hello guys, just wanted to let you know about an issue with typescript types for this module, when using moduleResolution: Node16
it causes some errors when trying to import the default exported fastify plugin callback, you can check this out here https://arethetypeswrong.github.io/?p=%40immobiliarelabs%2Ffastify-sentry%406.0.10
Go into Workflow jobs and remove ignore pattern to activate it
Looking at how Sentry is integrated in Express (https://github.com/getsentry/sentry-javascript/blob/master/packages/node/src/handlers.ts) I think we can pick up a thing or two and apply them here, for example:
shouldHandlerError
in the Express integration) to customize the event filtering instead of our array of statusCodesImplementing these changes would also resolve issues #5 and #3
My app uses JWT to authenticate users, which only contain a user ID, and not a user name.
The current requirement for getting extractUserData
to work (based on the types at least) is to always provide a username though:
Line 29 in 598a4dc
Is there a way to make them all optional? Not all my requests have an authenticated user (some are public routes) so being able to return 'nothing' would also come in quite handy.
Just an idea I had as I'm working through adding this to a project.
I am using a fastify server inside of AWS Lambda and thus already have a Sentry instance initialized. It would be nice to have an option to simply skip the Sentry initalization step to not confuse the library.
Currently all Sentry deps of fastify-sentry have strict versions:
"dependencies": {
"@sentry/node": "7.59.2",
"@sentry/tracing": "7.59.2",
"@sentry/utils": "7.59.2",
},
It means that if I need to install @sentry/node
to my project separatly to use some functions from it, I have to sync versions. If I use versions with caret (^7.60.0
), yarn installs all sentry deps in 2 places:
That causes type errors:
If we look to yarn.lock file, we could find that all Sentry version have duplicates with no caret:
So, my suggestion is to prefix all versions with caret:
"dependencies": {
"@sentry/node": "^7.59.2",
"@sentry/tracing": "^7.59.2",
"@sentry/utils": "^7.59.2",
},
Specifying strict resolution in package.json is a workaround
Issue
Hello. We started using your package to integrate Sentry into our Fastify applications. It worked really great until we integrated Sentry into another application which calls the first one.
I found that this is related to baggage
, hence why the issue only appears when Sentry is enabled in the caller app.
I then looked at Sentry's SDK code and found that the parseBaggageString()
function has been renamed to parseBaggageHeader()
in version 7.3.0.
Versions
node
: 16.18.1fastify
: 4.6.0@immobiliarelabs/fastify-sentry
: 5.0.0@sentry/*
: 7.18.0We're doing some hacky stuff to make Fastify work inside the main electron process. Somehow its hitting this line every time inside the electron process.
Line 145 in 598a4dc
When the Fastify server runs directly with node, it works as normal.
I'm thinking this is because this project uses direct imports of @sentry/node
and in our case we might need to use @sentry/electron
. Is there a way to change it or could you guys give us some help?
The error handler of the plugin uses a property on the error object to decide if it should redact the message. I think it's nice to check for the same property and add support for the plugin errors:
Just wanted to pop in and say thank you for making and maintaining this library :)
I see a bunch of recent commits to update deps which is quite exciting, particularly domain changes relating to Sentry 7.48!
Amazing job to everyone involved!
Is there a reason why Sentry versions are now pinned instead of using a ^
range? This seems to have changed in v5.0.1, just curious if that's actually needed.
The documentation cites fastify
as a parameter but it has been removed.
hey, i see that i need to import sentry transactions to be imported first then profiling import. how can i add profiling using the library?
I didn't want to use Sentry hooks to skip the event reporting for some status codes. I think we should benchmark the difference and consider using the default Sentry hooks if the drop isn't big. I think it's better to have a single point to handle the skipping, it's less confusing.
Test also the initialization options. Now we are just registering the plugin with the default options.
Attaach a hook to the onTimeout
hook of Fastify.
getsentry/sentry-javascript#6066
Pinning ti 7.16 temporarily to avoid breaking people npm install
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.
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:
declare module 'fastify' {
interface FastifyInstance {
Sentry?: typeof Sentry;
}
}
and then write fastify.Sentry?.addBreadcrumb
every time (with ?
).
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.
I couldn't make autoSessionTracking
work, opening this to remember to add support for it.
I have a route with a Knex instance throwing SQL errors. I use an error handler in my app.ts file to catch these and set a generic error message.
fastify.setErrorHandler((error, request, reply) => {
if ((process.env.NODE_ENV || 'development') === 'production') {
reply.status(500).send({ message: "Internal Server Error"});`
However, these thrown errors are coming back with a 2xx status code and a response body
{"error":500,"message":"Internal Server Error"}
fastify.withTypeProvider<ZodTypeProvider>().route({
method: "DELETE",
url: "/ingredients/:id",
onRequest: [fastify.authenticate, fastify.isAdmin],
schema: {
params: z.object({
id: z.string()
}),
},
handler: async (request, reply) => {
const ingredientId = Number(request.params.id);
await fastify.knex('ingredient')
.where('id', ingredientId)
.del();
reply.status(204).send();
},
});
This is what my route handler looks like.
And this is my Sentry plugin init
import fp from 'fastify-plugin';
export default fp(async (fastify, opts) => {
if (process.env.NODE_ENV === 'production') {
fastify.register(require('fastify-sentry'), {
dsn: process.env.SENTRY_DSN,
environment: process.env.NODE_ENV,
release: '1.0.0',
});
}
});
We are linting only during local development.It might be useful to do that in a CI job too.
This library has been great, but it's currently making it difficult to upgrade our Sentry library versions.
It seems like the problem has to do with a change to the definition of Sentry's NodeOptions
type.
Here's the typescript error output:
src/server.ts:41:3 - error TS2769: No overload matches this call.
Overload 1 of 3, '(plugin: FastifyPluginCallback<FastifySentryOptions, RawServerDefault, FastifyTypeProviderDefault>, opts?: FastifyRegisterOptions<...> | undefined): FastifyInstance<...> & PromiseLike<...>', gave the following error.
Argument of type 'NodeOptions' is not assignable to parameter of type 'FastifyRegisterOptions<FastifySentryOptions> | undefined'.
Type 'NodeOptions' is not assignable to type 'RegisterOptions & FastifySentryOptions'.
Type 'NodeOptions' is not assignable to type 'FastifySentryOptions'.
Types of property 'defaultIntegrations' are incompatible.
Type 'false | import("/Users/james/code/arcol/node_modules/@sentry/types/types/integration").Integration[] | undefined' is not assignable to type 'false | import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/integration").Integration[] | undefined'.
Type 'Integration[]' is not assignable to type 'false | Integration[] | undefined'.
Type 'import("/Users/james/code/arcol/node_modules/@sentry/types/types/integration").Integration[]' is not assignable to type 'import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/integration").Integration[]'.
Type 'import("/Users/james/code/arcol/node_modules/@sentry/types/types/integration").Integration' is not assignable to type 'import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/integration").Integration'.
Types of property 'setupOnce' are incompatible.
Type '(addGlobalEventProcessor: (callback: import("/Users/james/code/arcol/node_modules/@sentry/types/types/eventprocessor").EventProcessor) => void, getCurrentHub: () => import("/Users/james/code/arcol/node_modules/@sentry/types/types/hub").Hub) => void' is not assignable to type '(addGlobalEventProcessor: (callback: import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/eventprocessor").EventProcessor) => void, getCurrentHub: () => import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/typ...'.
Types of parameters 'addGlobalEventProcessor' and 'addGlobalEventProcessor' are incompatible.
Types of parameters 'callback' and 'callback' are incompatible.
Type 'import("/Users/james/code/arcol/node_modules/@sentry/types/types/eventprocessor").EventProcessor' is not assignable to type 'import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/eventprocessor").EventProcessor'.
Types of parameters 'event' and 'event' are incompatible.
Type 'import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/event").Event' is not assignable to type 'import("/Users/james/code/arcol/node_modules/@sentry/types/types/event").Event'.
Types of property 'spans' are incompatible.
Type 'import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/span").Span[] | undefined' is not assignable to type 'import("/Users/james/code/arcol/node_modules/@sentry/types/types/span").Span[] | undefined'.
Type 'import("/Users/james/code/arcol/node_modules/@immobiliarelabs/fastify-sentry/node_modules/@sentry/types/types/span").Span[]' is not assignable to type 'import("/Users/james/code/arcol/node_modules/@sentry/types/types/span").Span[]'.
Type 'Span' is missing the following properties from type 'Span': attributes, spanContext, end, setAttribute, and 3 more.
Overload 2 of 3, '(plugin: FastifyPluginAsync<FastifySentryOptions, RawServerDefault, FastifyTypeProviderDefault>, opts?: FastifyRegisterOptions<...> | undefined): FastifyInstance<...> & PromiseLike<...>', gave the following error.
Argument of type 'FastifySentryPlugin' is not assignable to parameter of type 'FastifyPluginAsync<FastifySentryOptions, RawServerDefault, FastifyTypeProviderDefault>'.
Overload 3 of 3, '(plugin: FastifyPluginCallback<FastifySentryOptions, RawServerDefault, FastifyTypeProviderDefault> | FastifyPluginAsync<...> | Promise<...> | Promise<...>, opts?: FastifyRegisterOptions<...> | undefined): FastifyInstance<...> & PromiseLike<...>', gave the following error.
Argument of type 'NodeOptions' is not assignable to parameter of type 'FastifyRegisterOptions<FastifySentryOptions> | undefined'.
41 server.register(FastifySentry, sentryOptions);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I haven't bisected @sentry/node
to figure out when exactly the breaking change was introduced, but it appears to have happened sometime between v7.75 and v7.105, the current version as of this issue's creation.
Let me know if I can provide more information that would be helpful here.
This filed has been deprecated, we should use setContext
instead of setExtra
.
I think it would be nice to expose an option to pass a custom function that can be used to enrich the event with custom data.
Hi there, thanks for building this useful integration!
I'm using Fastify with TRPC, which means my routes all match to a single request handler in fastify under something like /trpc/:rpcCall
. To break these routes out by their TRPC function call, I added a getTransactionName
:
getTransactionName: (request: FastifyRequest) => {
if (request.routerPath === '/trpc/:path') {
return (request.params as any)['path'];
}
return request.routerPath;
},
This works well, but because the source is type URL, Sentry treats these routes as a high cardinality value and groups them in this <<unparameterized>>
block
Happy to submit a fix, but wanted to get your thoughts on a few approaches:
1. Switch the current source value from url
to route
(here)
It seems to me like fastify's routerPath value should be treated as a route
source in sentry and not a url
source, since it returns urls like /users/:id
and not /users/1
. Relevant docs
2. Switch the current value of url
to custom
if the user provides a getTransactionName
function
(self explanatory)
3. Allow users to provide a custom getTransactionSource
Since the docs discourage users from doing this and say source
should only be specified by integrations, I'd probably suggest one of the former two.
Hi, thank you for your work, I love your integration.
I used this plugin in my app that is bundled into single file during build, but i stumbled on an obstacle.
During runtime, there si require
call for package.json
via relative path, which cannot be bundled at current state.
Is there a possibility to change this behavior to support code bundling?
I've tested one sollution. Changing
const PACKAGE_NAME = require(path.resolve(__dirname, 'package.json')).name;
to
const PACKAGE_NAME = require('./package.json').name;
would help :-)
Thanks!
Line 16 in 1838067
Avoid having a big array with all the status code and see if we can do some filtering based on strings, eg: 5xx
We can re-use the current scope of the request using:
Sentry.getCurrentHub().getScope()
instead of creating a new one with .withScope()
When a DSN is not passed, the plugin logs a message which implies that the init
function of the Sentry SDK will not be called and that's not true.
Ref #93 .
Hi guys, is there any plan to update deps to support fastify v4 any time soon?
Would be great to test it.
Have a great day!
Hey, I'm not sure what would be the best approach for this but I don't think the current status quo is correct either.
Let's say there's a route
fastify.get('/someroute', {
schema: {
querystring: {
type: 'object',
properties: {
dob: { type: 'string', format: 'date' },
},
required: ['dob']
}
}
}, (req, res) => {
return res.send('foo')
})
Now, if with fastify-sentry I try to call this route with an invalid date of birth, I'm getting {"statusCode":500,"error":"Internal Server Error","message":"Something went wrong"}
. Without fastify-sentry I get the more sensible {"statusCode":400,"error":"Bad Request","message":"querystring/dob must match format \"date\""}
IMO these format errors should be fully excluded from fastify-sentry and returned to the client as is.
Seems like one approach (described here https://www.fastify.io/docs/latest/Reference/Validation-and-Serialization/#validation-messages-with-other-validation-libraries) would be to check if validation
property exists on the error, and in those cases let it pass through
This looks like a great wrapper for Sentry + Fastify. I'd love to use it in my project.
I don't know if I'm missing an important step in the instructions or something, but I cannot get this to work at all:
import fastifySentry from '@immobiliarelabs/fastify-sentry'
await fastify.register(fastifySentry, {
dsn: process.env.SENTRY_DSN,
debug: true,
environment: process.env.SERVER_ENV,
tracesSampleRate: 1.0,
release: '1.0.0',
})
fastify.route({
method: 'GET',
url: '/test',
handler: async (request, reply) => {
throw new Error('test1234')
},
})
I have the following issues:
captureException
in base.js
is firing, but nothing is showing up in Sentry.fastify.Sentry.captureMessage()
doesn't work eithertracesSampleRate: 1.0
does not work because if (hasTracingEnabled()) {
in request.js
evaluates to false.I've ensured my SENTRY_DSN is correct, as it works when I use the Sentry Node SDK, and double checked your examples and I don't think I'm missing anything.
Any ideas would be appreciated!
In its latest versions Sentry has added tracing performance capabilities, it would be nice to be able to support that, using an option to enable/disable the functionality or maybe using a separate plugin ๐ค
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.