338f08b3a412ca522c66e267238f623bbe7b0d2f
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Marie
|
362d540aac |
Misc. of sentry improvements (#12233)
This PR mixes various initiatives to improve visibility on sentry **1. Catch errors on workflow jobs** commit [catch workflowTriggerExceptions in job handle](https://github.com/twentyhq/twenty/commit/1dbba8c9e27fed8fe01c5ac91d492720c45ba7a5) @thomtrp **2. Fix type in messagingImportExceptionHandler** commit [fix type issue on messagingImportExceptionHandler](https://github.com/twentyhq/twenty/commit/919bb3844c2e11f929a01dff0539e0a2ec7e4b00) @guillim **3. Catch invalid uuid errors thrown by Postgres by rightfully typing expected id as uuid** commits [use UUIDFilter instead of IDFilter to get graphqlError in case of malformed id](https://github.com/twentyhq/twenty/commit/57cc315efeb0365b1cd589f7487dbfc886338d26), [use UUIDFilter (2)](https://github.com/twentyhq/twenty/commit/304553d770426362ee8bfcd3c2e1f0066abe2c04), [fix ids typed as UUID instead of ID](https://github.com/twentyhq/twenty/commit/f95d6319cf9290a5ae1d59eb634617a511411633) @Weiko ⚠️⚠️⚠️ when we deploy this PR we need to flush the schema types from redis as this PR changes them ⚠️⚠️⚠️ **4. Do not group UNKNOWN errors together** commit [do not group unknown errors together](https://github.com/twentyhq/twenty/commit/c299b39c8fdcc4d6656dc5ce58437e4365cd60b0) Some CustomException classes have introduced UNKNOWN error codes as a default fallback error code. We use CustomException codes to group issues together, but we don't want to do it with UNKNOWN error as they may not have anything in common. For exemple [this sentry for UNKNOWN code](https://twenty-v7.sentry.io/issues/6605750776/events/a72272d8941b4fa2add9b1f39c196d3f/?environment=prod&environment=prod-eu&project=4507072499810304&query=Unknown&referrer=next-event&stream_index=0) groups together "Unknown error importing calendar events for calendar channel...", "Insufficent permissions...", to name a few. **5. Improve postgres error grouping** commit [group together postgres errors](https://github.com/twentyhq/twenty/commit/567c25495e37cdccfc9010f346145f7d5cc46a54) Postgres error are thrown by typeORM as QueryFailedError. we have a lot of them on sentry where they are badly grouped They are currently grouped on sentry according to the stack trace, which leads them to sometimes be grouped even if they don't have anything in common : for exemple [this sentry for QueryFailedError](https://twenty-v7.sentry.io/issues/6563624590/events/2d636821e27a448595b647b4b5a7d6a8/?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=previous-event&sort=date&stream_index=0) groups together "user mapping not found for "postgres" and "invalide type for uuid: 'fallback-id'" to name a few. I attempted to improve the grouping by grouping them with a new custom fingerPrint composed of the [code returned by Postgres](https://www.postgresql.org/docs/current/errcodes-appendix.html) + the truncated operation name (Find, Aggregate, Check...). This is still not ideal as postgres code are quite broad - we could have the same error code for two Find operations with different causes. let's give this a try ! |
||
|
Paul Rastoin
|
9ad8287dbc |
[REFACTOR] twenty-shared multi barrel and CJS/ESM build with preconstruct (#11083)
# Introduction In this PR we've migrated `twenty-shared` from a `vite` app [libary-mode](https://vite.dev/guide/build#library-mode) to a [preconstruct](https://preconstruct.tools/) "atomic" application ( in the future would like to introduce preconstruct to handle of all our atomic dependencies such as `twenty-emails` `twenty-ui` etc it will be integrated at the monorepo's root directly, would be to invasive in the first, starting incremental via `twenty-shared`) For more information regarding the motivations please refer to nor: - https://github.com/twentyhq/core-team-issues/issues/587 - https://github.com/twentyhq/core-team-issues/issues/281#issuecomment-2630949682 close https://github.com/twentyhq/core-team-issues/issues/589 close https://github.com/twentyhq/core-team-issues/issues/590 ## How to test In order to ease the review this PR will ship all the codegen at the very end, the actual meaning full diff is `+2,411 −114` In order to migrate existing dependent packages to `twenty-shared` multi barrel new arch you need to run in local: ```sh yarn tsx packages/twenty-shared/scripts/migrateFromSingleToMultiBarrelImport.ts && \ npx nx run-many -t lint --fix -p twenty-front twenty-ui twenty-server twenty-emails twenty-shared twenty-zapier ``` Note that `migrateFromSingleToMultiBarrelImport` is idempotent, it's atm included in the PR but should not be merged. ( such as codegen will be added before merging this script will be removed ) ## Misc - related opened issue preconstruct https://github.com/preconstruct/preconstruct/issues/617 ## Closed related PR - https://github.com/twentyhq/twenty/pull/11028 - https://github.com/twentyhq/twenty/pull/10993 - https://github.com/twentyhq/twenty/pull/10960 ## Upcoming enhancement: ( in others dedicated PRs ) - 1/ refactor generate barrel to export atomic module instead of `*` - 2/ generate barrel own package with several files and tests - 3/ Migration twenty-ui the same way - 4/ Use `preconstruct` at monorepo global level ## Conclusion As always any suggestions are welcomed ! |
||
|
Marie
|
861face2a8 |
[permissions] Enforce object-records permission checks in resolvers (#10304)
Closes https://github.com/twentyhq/core-team-issues/issues/393 - enforcing object-records permission checks in resolvers for now. we will move the logic to a lower level asap - add integration tests that will still be useful when we have moved the logic - introduce guest seeded role to test limited permissions on object-records |