Commit Graph
24 Commits
Author SHA1 Message Date
MarieandGitHub 635300b1dd Fix error handling (#13072)
Fixes
https://twenty-v7.sentry.io/issues/6730499496/?environment=prod&environment=prod-eu&project=4507072499810304&query=is%3Aunresolved%20%21issue.type%3A%5Bperformance_consecutive_db_queries%2Cperformance_consecutive_http%2Cperformance_file_io_main_thread%2Cperformance_db_main_thread%2Cperformance_n_plus_one_db_queries%2Cperformance_n_plus_one_api_calls%2Cperformance_p95_endpoint_regression%2Cperformance_slow_db_query%2Cperformance_render_blocking_asset_span%2Cperformance_uncompressed_assets%2Cperformance_http_overhead%2Cperformance_large_http_payload%5D%20timesSeen%3A%3E10&referrer=issue-stream&sort=date&stream_index=0
2025-07-07 14:22:54 +02:00
MarieandGitHub 288f0919db Define server error messages to display in FE from the server (#12973)
Currently, when a server query or mutation from the front-end fails, the
error message defined server-side is displayed in a snackbar in the
front-end.
These error messages usually contain technical details that don't belong
to the user interface, such as "ObjectMetadataCollection not found" or
"invalid ENUM value for ...".

**BE**
In addition to the original error message that is still needed (for the
request response, debugging, sentry monitoring etc.), we add a
`displayedErrorMessage` that will be used in the snackbars. It's only
relevant to add it for the messages that will reach the FE (ie. not in
jobs or in rest api for instance) and if it can help the user sort out /
fix things (ie. we do add displayedErrorMessage for "Cannot create
multiple draft versions for the same workflow" or "Cannot delete
[field], please update the label identifier field first", but not
"Object metadata does not exist"), even if in practice in the FE users
should not be able to perform an action that will not work (ie should
not be able to save creation of multiple draft versions of the same
workflows).

**FE**
To ease the usage we replaced enqueueSnackBar with enqueueErrorSnackBar
and enqueueSuccessSnackBar with an api that only requires to pass on the
error.
If no displayedErrorMessage is specified then the default error message
is `An error occured.`
2025-07-03 12:42:10 +00:00
MarieandGitHub 2d5312276c Improve FE error handling (#12864)
This PR aims at improving readability in sentry and user experience with
runtime errors.

**GraphQL errors (and ApolloError)**
1. In sentry we have a lot of "Object captured as exception with keys:
extensions, message" errors (2k over the last 90d), on which we have
zero information. This is because in apollo-factory we were passing on
GraphQL errors to sentry directly why sentry expects the structure of a
JS Error. We are now changing that, rebuilding an Error object and
attempting to help grouping by creating a fingerPrint based on error
code and truncated operationName (same as we do in the back for 500
graphql errors).
2. In sentry we have a lot of ApolloError, who actually correspond to
errors that should not be logged in sentry (Forbidden errors such as
"Email is not verified"), or errors that are already tracked by back-end
(Postgres errors such as "column xxx does not exist"). This is because
ApolloErrors become unhandled rejections errors if they are not caught
and automatically sent to sentry through the basic config. To change
that we are now filtering out ApolloErrors created from GraphQL Errors
before sending error to sentry:
<img width="524" alt="image"
src="https://github.com/user-attachments/assets/02974829-26d9-4a9e-8c4c-cfe70155e4ab"
/>

**Runtime errors**
4. Runtime errors were all caught by sentry with the name "Error",
making them not easy to differentiate on sentry (they were not grouped
together but all appeared in the list as "Error"). We are replacing the
"Error" name with the error message, or the error code if present. We
are introducing a CustomError class that allows errors whose message
contain dynamic text (an id for instance) to be identified on sentry
with a common code. _(TODO: if this approach is validated then I have
yet to replace Error with dynamic error messages with CustomError)_
5. Runtime error messages contain technical details that do not mean
anything to users (for instance, "Invalid folder ID: ${droppableId}",
"ObjectMetadataItem not found", etc.). Let's replace them with "Please
refresh the page." to users and keep the message error for sentry and
our dev experience (they will still show in the console as uncaught
errors).
2025-06-26 15:54:12 +00:00
b31845b7ba Fix resolver-validation validation snake trap (#12850)
# Introduction
This PR might have a lot of impact on tested validation
Avoid catching programmatically thrown error

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
2025-06-24 18:30:56 +02:00
WeikoandGitHub ed11cac5f7 Add graphql queries error codes metrics (#12833)
## Context
Added to the existing useGraphQLErrorHandlerHook yoga hook to increment
metrics after all query executions based on their error codes. I
originally wanted to create a new useMetrics hook but most of the error
handling was done in useGraphQLErrorHandlerHook so we decided to keep it
there for now.

<img width="1310" alt="Screenshot 2025-06-24 at 15 58 26"
src="https://github.com/user-attachments/assets/498d3754-851a-4051-a5c2-23ac8253aa6a"
/>
2025-06-24 16:13:33 +02:00
Charles BochetandGitHub b76dac2ca1 BREAKING CHANGE: Fix graphql errors (#12775)
We were using a global ValidationPipe in main.ts. This is an issue as
@Controllers should return HttpExecption and @Resolvers should return
GraphqlErrors

Removing the global pipe and creating a ResolverValidationPipe able to
generate GraphqlError. We also need to handle the exception in a filter
to avoid nest to think it's unhandled and make it flow to logs


Next step:
- it would be nice to have both @UsePipes(ResolverValidationPipe) +
@UseFilters(GraphqlValidationExceptionFilter) come together. This should
be possible if we create a @GraphQLResolver annotation
2025-06-23 11:23:16 +02:00
5428348d7f fix(email-verification): prevent double email validation (#12250)
Fix #12177 
Fix #12171

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
2025-05-23 16:24:26 +00:00
WeikoandGitHub 806bb611e8 Fix yoga scalar validations being captured (#12085)
Yoga graphql error were not correctly interpreted by the exception
handler. Mostly validations on the scalars such as bad enum options,
wrong format for uuid and such.
This PR adds a new convertGraphQLErrorToBaseGraphQLError utility
function in graphql-errors.util.ts that converts those errors to our
custom BaseGraphQLError by using the extension.http.code from the error
when possible so they can be handled the same way we treat the graphql
errors we throw ourselves.

Before
<img width="799" alt="Screenshot 2025-05-16 at 11 04 08"
src="https://github.com/user-attachments/assets/08b0a908-34d8-45a6-b315-8e211d1104ce"
/>

After
<img width="797" alt="Screenshot 2025-05-16 at 11 16 37"
src="https://github.com/user-attachments/assets/3fff0a70-6c3f-413a-b458-56030377fec9"
/>
2025-05-16 18:11:52 +02:00
MarieandGitHub dc4bcc3049 Improve sentry filtering and grouping (#12071)
Follow-up on https://github.com/twentyhq/twenty/pull/12007

In this PR

- adding a filter on HttpExceptionHandlerService to filter out 4xx
errors from driver handling (as we do for graphQL errors: see
useGraphQLErrorHandler hook - only filteredIssues are sent to`
exceptionHandlerService.captureExceptions()`.)
- grouping together more missing metadata issues
- attempting to use error codes as issues names in sentry to improve UI;
for now it says "Error" all the time
2025-05-16 11:35:48 +02:00
Paul RastoinandGitHub 442f8dbe3c [QRQC_2] No implicitAny in twenty-server (#12075)
# Introduction
Following https://github.com/twentyhq/twenty/pull/12068
Related with https://github.com/twentyhq/core-team-issues/issues/975

We're enabling `noImplicitAny` handled few use case manually, added a
`ts-expect-error` to the others, we should plan to handle them in the
future
2025-05-15 18:23:22 +02:00
a8423e8503 [QRQC_2] No explicit any in twenty-server (#12068)
# Introduction

Added a no-explicit-any rule to the twenty-server, not applicable to
tests and integration tests folder

Related to https://github.com/twentyhq/core-team-issues/issues/975
Discussed with Charles

## In case of conflicts
Until this is approved I won't rebased and handle conflict, just need to
drop two latest commits and re run the scripts etc

## Legacy
We decided not to handle the existing lint error occurrences and
programmatically ignored them through a disable next line rule comment

## Open question
We might wanna activate the
[no-explicit-any](https://typescript-eslint.io/rules/no-explicit-any/)
`ignoreRestArgs` for our use case ?
```
    ignoreRestArgs?: boolean;
```

---------

Co-authored-by: etiennejouan <jouan.etienne@gmail.com>
2025-05-15 16:26:38 +02:00
9c2b88870f Improve sentry grouping (#12007)
This PR attemps at improving sentry grouping and filtering by 
- Using the exceptionCode as the fingerprint when the error is a
customException. For this to work in this PR we are now throwing
customExceptions instead of internalServerError deprived of their code.
They will still be converted to Internal server errors when sent back as
response
- Filtering 4xx issues where it was missing (for emailVerification
because errors were not handled, for invalid captcha and billing errors
because they are httpErrors and not graphqlErrors)

---------

Co-authored-by: Félix Malfait <felix@twenty.com>
2025-05-14 09:00:06 +00:00
Gitesh SarvaiyaandGitHub 48011ac012 fix: do not report EmailNotVerifiedError to Sentry (#11753) (#11835)
This PR prevents EmailNotVerifiedError from being reported to Sentry, as
discussed in issue #11753.

edit:
close #11753
2025-05-02 15:35:05 +02:00
Félix MalfaitandGitHub ab277476a8 Remove Sentry fingerprint (#11602)
As discussed this @ijreilly, Fingerprinting is probably not needed and
Sentry will do a better job by itself
2025-04-16 16:25:40 +02:00
nitinandGitHub bd3ec6d5e3 rename core-module environment to twenty-config (#11445)
closes https://github.com/twentyhq/core-team-issues/issues/759
2025-04-09 14:11:26 +02:00
Félix MalfaitandGitHub 8df59c085d Lingui working with NODE ENV=production again (#10067)
Lingui now offers an option to disable stripping even in prod mode so we
can bring it back
2025-02-07 10:05:07 +01:00
f722a2d619 Add Email Verification for non-Microsoft/Google Emails (#9288)
Closes twentyhq/twenty#8240 

This PR introduces email verification for non-Microsoft/Google Emails:

## Email Verification SignInUp Flow:

https://github.com/user-attachments/assets/740e9714-5413-4fd8-b02e-ace728ea47ef

The email verification link is sent as part of the
`SignInUpStep.EmailVerification`. The email verification token
validation is handled on a separate page (`AppPath.VerifyEmail`). A
verification email resend can be triggered from both pages.

## Email Verification Flow Screenshots (In Order):

![image](https://github.com/user-attachments/assets/d52237dc-fcc6-4754-a40f-b7d6294eebad)

![image](https://github.com/user-attachments/assets/263a4b6b-db49-406b-9e43-6c0f90488bb8)

![image](https://github.com/user-attachments/assets/0343ae51-32ef-48b8-8167-a96deb7db99e)

## Sent Email Details (Subject & Template):
![Screenshot 2025-01-05 at 11 56
56 PM](https://github.com/user-attachments/assets/475840d1-7d47-4792-b8c6-5c9ef5e02229)

![image](https://github.com/user-attachments/assets/a41b3b36-a36f-4a8e-b1f9-beeec7fe23e4)

### Successful Email Verification Redirect:

![image](https://github.com/user-attachments/assets/e2fad9e2-f4b1-485e-8f4a-32163c2718e7)

### Unsuccessful Email Verification (invalid token, invalid email, token
expired, user does not exist, etc.):

![image](https://github.com/user-attachments/assets/92f4b65e-2971-4f26-a9fa-7aafadd2b305)

### Force Sign In When Email Not Verified:

![image](https://github.com/user-attachments/assets/86d0f188-cded-49a6-bde9-9630fd18d71e)

# TODOs:

## Sign Up Process

- [x] Introduce server-level environment variable
IS_EMAIL_VERIFICATION_REQUIRED (defaults to false)
- [x] Ensure users joining an existing workspace through an invite are
not required to validate their email
- [x] Generate an email verification token
- [x] Store the token in appToken
- [x] Send email containing the verification link
  - [x] Create new email template for email verification
- [x] Create a frontend page to handle verification requests

## Sign In Process

- [x] After verifying user credentials, check if user's email is
verified and prompt to to verify
- [x] Show an option to resend the verification email

## Database

- [x] Rename the `emailVerified` colum on `user` to to `isEmailVerified`
for consistency

## During Deployment
- [x] Run a script/sql query to set `isEmailVerified` to `true` for all
users with a Google/Microsoft email and all users that show an
indication of a valid subscription (e.g. linked credit card)
- I have created a draft migration file below that shows one possible
approach to implementing this change:

```typescript
import { MigrationInterface, QueryRunner } from 'typeorm';

export class UpdateEmailVerifiedForActiveUsers1733318043628
  implements MigrationInterface
{
  name = 'UpdateEmailVerifiedForActiveUsers1733318043628';

  public async up(queryRunner: QueryRunner): Promise<void> {
    await queryRunner.query(`
      CREATE TABLE core."user_email_verified_backup" AS
      SELECT id, email, "isEmailVerified"
      FROM core."user"
      WHERE "deletedAt" IS NULL;
    `);

    await queryRunner.query(`
      -- Update isEmailVerified for users who have been part of workspaces with active subscriptions
      UPDATE core."user" u
      SET "isEmailVerified" = true
      WHERE EXISTS (
        -- Check if user has been part of a workspace through userWorkspace table
        SELECT 1 
        FROM core."userWorkspace" uw
        JOIN core."workspace" w ON uw."workspaceId" = w.id
        WHERE uw."userId" = u.id
        -- Check for valid subscription indicators
        AND (
          w."activationStatus" = 'ACTIVE'
          -- Add any other subscription-related conditions here
        )
      )
      AND u."deletedAt" IS NULL;
  `);
  }

  public async down(queryRunner: QueryRunner): Promise<void> {
    await queryRunner.query(`
      UPDATE core."user" u
      SET "isEmailVerified" = b."isEmailVerified"
      FROM core."user_email_verified_backup" b
      WHERE u.id = b.id;
    `);

    await queryRunner.query(`DROP TABLE core."user_email_verified_backup";`);
  }
}

```

---------

Co-authored-by: Antoine Moreaux <moreaux.antoine@gmail.com>
Co-authored-by: Félix Malfait <felix@twenty.com>
2025-01-15 18:43:40 +01:00
martmullandGitHub b10d831371 8726 workflow add a test button in workflow code step (#9016)
- add test button to workflow code step
- add test tab to workflow code step


https://github.com/user-attachments/assets/e180a827-7321-49a2-8026-88490c557da2



![image](https://github.com/user-attachments/assets/cacbd756-de3f-4141-a84c-8e1853f6556b)

![image](https://github.com/user-attachments/assets/ee170d81-8a22-4178-bd6d-11a0e8c73365)
2024-12-13 10:16:29 +00:00
GuillimandGitHub 7e4277fbe4 Better errors handling (#8835)
- [ ] Catch this specific `500` error
- [ ] Make sure catched `500` errors are sent to sentry for the Cloud
version
- [ ] Hide the option to sync email with google in this situation where
the according env var is missing
- [x] Add Worskpace information to all catched errors for better
debugging

fix #8607
2024-12-03 12:16:38 +01:00
martmullandGitHub 3190f4a87b 6658 workflows add a first twenty piece email sender (#6965) 2024-09-12 11:00:25 +02:00
Charles BochetandGitHub 17a1760afd Improve performance twenty orm (#6691)
## Context

As we grow, the messaging scripts are experiencing performance issues
forcing us to temporarily disable them on the cloud.
While investigating the performance, I have noticed that generating the
entity schema (for twentyORM) in the repository is taking ~500ms locally
on my Mac M2 so likely more on pods. Caching the entitySchema then!

I'm also clarifying naming around schemaVersion and cacheVersions ==>
both are renamed workspaceMetadataVersion and migrated to the workspace
table (the workspaceCacheVersion table is dropped).
2024-08-20 19:42:02 +02:00
Thomas TrompetteandGitHub 68e3730be1 Fix base graphql error message (#6457)
Display error message instead of name

<img width="762" alt="Capture d’écran 2024-07-30 à 15 51 07"
src="https://github.com/user-attachments/assets/a6b49254-a99e-42fc-8704-089f38bf8e7b">
2024-07-30 15:55:57 +02:00
515d6fb1c5 Filter out by error code + invert filtering (#6432)
As title

Instance of not working well.

Co-authored-by: Charles Bochet <charles@twenty.com>
2024-07-29 16:18:23 +02:00
1dff5bf957 Fix custom errors thrown as 500 (#6238)
We call convertExceptionToGraphQLError in the exception handler for http
exceptions but we don't take into account those that already are
graphqlErrors and because of that the logic of convertExceptionToGraphql
is to fallback to a 500.
Now if the exception is a BaseGraphqlError (custom graphql error we
throw in the code), we throw them directly.

BEFORE
<img width="957" alt="Screenshot 2024-07-12 at 15 33 03"
src="https://github.com/user-attachments/assets/22ddae13-4996-4ad3-8f86-dd17c2922ca8">


AFTER
<img width="923" alt="Screenshot 2024-07-12 at 15 32 01"
src="https://github.com/user-attachments/assets/d3d6db93-6d28-495c-a4b4-ba4e47d45abd">

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
2024-07-12 17:56:21 +02:00