Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server crash if 500 is thrown #8607

Open
guillim opened this issue Nov 20, 2024 · 5 comments
Open

Server crash if 500 is thrown #8607

guillim opened this issue Nov 20, 2024 · 5 comments
Assignees

Comments

@guillim
Copy link
Contributor

guillim commented Nov 20, 2024

Bug Description

Context

I found out this issue locally running on macOS the command npx nx run twenty-server:start
I was trying to sync emails on a setup where the google env var were not defined.

IS_SIGN_UP_DISABLED=false
# AUTH_MICROSOFT_ENABLED=false
# AUTH_MICROSOFT_CLIENT_ID=replace_me_with_azure_client_id
# AUTH_MICROSOFT_TENANT_ID=replace_me_with_azure_tenant_id
# AUTH_MICROSOFT_CLIENT_SECRET=replace_me_with_azure_client_secret
# AUTH_MICROSOFT_CALLBACK_URL=https://localhost:3000/auth/microsoft/redirect
# AUTH_GOOGLE_ENABLED=true
# AUTH_GOOGLE_CLIENT_ID=replace_me_with_google_client_id
# AUTH_GOOGLE_CLIENT_SECRET=replace_me_with_google_client_secret
# AUTH_GOOGLE_CALLBACK_URL=https://localhost:3000/auth/google/redirect
# AUTH_GOOGLE_APIS_CALLBACK_URL=https://localhost:3000/auth/google-apis/get-access-token
# AUTH_SSO_ENABLED=true

Image

The bug

The server crashes, and no other request can be handled by the server until it is manually restarted.

/Users/gl/Documents/twenty/packages/twenty-server/src/engine/core-modules/auth/filters/auth-rest-api-exception.filter.ts:26
        throw new UnauthorizedException(exception.message);
              ^
UnauthorizedException: Google apis auth is not enabled
    at AuthRestApiExceptionFilter.catch (/Users/gl/Documents/twenty/packages/twenty-server/src/engine/core-modules/auth/filters/auth-rest-api-exception.filter.ts:26:15)
    at ExceptionsHandler.invokeCustomFilters (/Users/gl/Documents/twenty/node_modules/@nestjs/core/exceptions/exceptions-handler.js:33:26)
    at ExceptionsHandler.next (/Users/gl/Documents/twenty/node_modules/@nestjs/core/exceptions/exceptions-handler.js:13:18)
    at /Users/gl/Documents/twenty/node_modules/@nestjs/core/router/router-proxy.js:13:35
    at processTicksAndRejections (node:internal/process/task_queues:95:5)```

Expected behavior

  • First of all, we should not display the option to sync email with google in this situation where the env var are missing.
  • Second, when the server handles this 500 error, it should catch it and return a more appropriate message.
  • Finally, the server should not freeze like that.
  • Bonus, we could keep track of these errors in sentry

To be discussed further with @charlesBochet

@charlesBochet
Copy link
Member

Bumping this one to prio high

@myspace20
Copy link

I will like to work on this issue.

@guillim
Copy link
Contributor Author

guillim commented Nov 25, 2024

Did you find out anything @myspace20 ?

@myspace20
Copy link

I was a little busy. I will find some time to look at it again.

@guillim guillim self-assigned this Nov 25, 2024
@guillim
Copy link
Contributor Author

guillim commented Nov 25, 2024

Technical suggestion

  • 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

FurtherMore

  • Investigate why server crashes entirely for this kind of error

@charlesBochet charlesBochet moved this from 🆕 New to 🔖 Planned in Product development ✅ Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔖 Planned
Development

No branches or pull requests

3 participants