twentyhq / twenty

Building a modern alternative to Salesforce, powered by the community.
https://twenty.com
GNU Affero General Public License v3.0
15.88k stars 1.75k forks source link

Identify users in Sentry Backend #3186

Closed FelixMalfait closed 7 months ago

FelixMalfait commented 8 months ago

Scope & Context

When we capture exceptions, we are sending them to our Exception Handler. See ExceptionHandlerService / captureException These errors are sent to Sentry in production. They are working fine but we would like to know which user (and on which workspace) the error has been triggered. We would like to add user_id, workspaceMember_id, workspace_id to this payload when they are available (obviously if an error happens while the user is logged out, we cannot send them) This has already been done on the frontend.

Technical inputs

See: https://docs.sentry.io/platforms/node/enriching-events/identify-user/ On the BE, the user information should be computed based on JWT token. Exceptions are handled here: global-exception-handler.util.ts. I am not sure about the best way to properly make the user available in this file, we might have to rather pass it as a parameter of globalExceptionHandler when we call it from GraphQLConfigService for example. See all the services calling globalExceptionHandler, we should make sure the user is passed in all cases.

mohamedamara1 commented 8 months ago

Links I found related to this :

Also this :

charlesBochet commented 7 months ago

This has been implemented, closing this issue

sbriceland commented 1 month ago

@charlesBochet If this has been implemented, then there still is a bug. As @FelixMalfait mentioned, the documentation is misleading.

From Scopes Documentation:

For instance, a web server might handle multiple requests at the same time, and each request may have different scope data to apply to its events.

The isolation scope is used to isolate events from each other. For example, each request in a web server might get its own isolation scope, so that events from one request don't interfere with events from another request. In most cases, you'll want to put data that should be applied to your events on the isolation scope - which is also why all Sentry.setXXX methods, like Sentry.setTag(), will write data onto the currently active isolation scope. A classic example for data that belongs on the isolation scope is a user - each request may have a different user, so you want to make sure that the user is set on the isolation scope

... The documentation matches exactly the behavior needed. However, it simply does not work.

Problems

This example using Express should demonstrate...

import * as Sentry from '@sentry/node';
import bodyParser from 'body-parser';
import express, { Application, NextFunction, Request, Response, Router } from 'express';

Sentry.init({
    dsn: undefined, // not needed for local debugging to reveal the issue
    beforeSend: (event, hint, ...args) => {
        const { type, contexts, exception, extra, tags, message, user, request } = event;
        console.dir(
            {
                whoami: 'sentry:beforeSend',
                event: { type, contexts, exception, extra, tags, message, user, request },
                hint,
                args
            },
            { depth: null }
        );
        return event;
    },
    skipOpenTelemetrySetup: true
});

const app: Application = express();

const router = Router();

router.use(bodyParser.urlencoded({ extended: true, limit: '500kb' }));
router.use(bodyParser.json({ limit: '500kb' }));

const Users: { id: string; email: string; name: string }[] = [
    { id: '1', email: 'foo@example.com', name: 'foo example' },
    { id: '2', email: 'foo2@example.com', name: 'foo example2' },
    { id: '3', email: 'foo3@example.com', name: 'foo example3' },
    { id: '4', email: 'foo4@example.com', name: 'foo example4' }
];

router.use('/users', function (req, res, next) {
    try {
        const authUser = Users.find((u) => u.id === req.headers['authorization']);
        if (authUser) {
            Sentry.setTag('Authenticated', true);
            Sentry.setUser(authUser);
            res.json(Users);
        } else {
            throw new Error('Authentication Error');
        }
    } catch (err) {
        next(err);
    }
});

app.use('/api', router);

app.use(function (err: Error, req: Request, res: Response, next: NextFunction) {
    const { method, originalUrl, params, query, body } = req;
    const { statusCode, locals } = res;

    Sentry.withScope((scope) => {
        scope.setExtras({
            request: { method, originalUrl, params, query, body },
            response: { statusCode, locals }
        });
        const eventId = Sentry.captureException(err);
        (res as { sentry?: string }).sentry = eventId;
    });

    next(err);
});

// Or just use this, which is identical to above, without `extras`
// Sentry.setupExpressErrorHandler(app);

const PORT = process.env.PORT || 3000
app.listen(PORT, () => {
    console.log(`API running @ http://localhost:${PORT}`);
});

Repro Steps:

Send the following requests:

# make an "authenticated" request
curl --header "authorization: 1" --header "content-type: application/json" http://localhost:3000/api/users

# subsequently make an unauthenticated request
curl --header "authorization: 798798798798" --header "content-type: application/json" http://localhost:3000/api/users
sbriceland commented 1 month ago

oh dear .. sorry folks. I thought i was over on the Sentry issue page. Nothing to see here. I'll move my issue to them

FelixMalfait commented 1 month ago

@sbriceland no worries! I guess our repo is now ranking well on Google 😅