From Sentry: The singular and plural name cannot be the same for an object

[sentry-io[bot]]

Context

This shouldn't throw an internal error, instead we should return a user-friendly message.

Sentry

Error: The singular and plural name cannot be the same for an object
  File "/app/packages/twenty-server/dist/src/metadata/object-metadata/object-metadata.service.js", line 44, in ObjectMetadataService.createOne
    throw new Error('The singular and plural name cannot be the same for an object');

[Souravpakhira]

hey can I fix this issue?

[charlesBochet]

@Souravpakhira sure! 1) actually this error should not be sent to Sentry it's a normal behavior. In our globalExceptionHandler, this error should have a code lower than 500. (It's actually a Bad Request exception that should be a 400) 2) On the FE side we should prevent this from happening. On Object creation page, we would need to display an error below the singular and plural name fields if both are equal

@FelixMalfait what do you mean by more user-friendly? This message is returned while we try to create an object with the same singular and plural.

[FelixMalfait]

@charlesBochet by more user-friendly I mostly meant a 400 and not a 500! I expected 500 to be displayed a bit differently but if not then it's just about changing error code and making sure it isn't sent to Sentry!

@charlesBochet on my side I'm not too keen on this kind of frontend error catching that will appear as you type (because you need to go through typing the singular to type the plural). But letting @Bonapara weight in

@Souravpakhira assigning you, let us know if you end up being blocked! Thanks a lot

[Souravpakhira]

@charlesBochet In this situation, should I address the issue at both the frontend and backend levels? This involves preventing the frontend from sending incorrect data, and on the backend, responding with a 400 error instead of a 500 error.

[Souravpakhira]

@charlesBochet by more user-friendly I mostly meant a 400 and not a 500! I expected 500 to be displayed a bit differently but if not then it's just about changing error code and making sure it isn't sent to Sentry!

@charlesBochet on my side I'm not too keen on this kind of frontend error catching that will appear as you type (because you need to go through typing the singular to type the plural). But letting @Bonapara weight in

@Souravpakhira assigning you, let us know if you end up being blocked! Thanks a lot

Reading through your comment I will fix the backend and let you guys decide If it needs to be handled from frontend as well

[FelixMalfait]

@Souravpakhira Perfect thanks! Let's do backend only for now. We'll create another issue if frontend is needed (we don't really have a design to display errors nicely if we wanted to do it on the frontend)

[Souravpakhira]

@FelixMalfait sure thanks I prioritize the backend work for now.

[charlesBochet]

@Souravpakhira @FelixMalfait OK!

Adding a few more hints: Regarding the backend, we are in the context of GraphQL here, so we actually return a 200 containing the error.

Take a look at global-exception-handler.util.ts, it does two things:

• deciding if we should send the error to sentry of not (here if we trigger a BadRequestException instead of an Error, this should stop sending it to Sentry)
• parsing it properly to be returned in Graphql payload (I think it's the first time we are parsing a 400)

Also, I am sure global-exception-handler.util.ts is called in the case of our /graphql endpoint. Here we are on /metadata endpoint. I think it should be called by our GlobalExceptionFilter that should cover all our nestJS endpoints but I have never tested it, it might not be.

Good luck and thank you!

[Souravpakhira]

@charlesBochet Thank you for sharing your insights. I'll make sure to address all the points you've mentioned. If I encounter any challenges, I'll reach out to you on Discord for guidance.

[Souravpakhira]

hey are we looking for something like this solution link this is wrt to point 2 @charlesBochet ? If yes than I guess it will bring alot of changes in backend and frontend how we query the data.

[charlesBochet]

It's a good question, I thought it was actually supported by our backend out of the box. We should take a look when @magrinj is back

[Souravpakhira]

Sure no problem till than I will look for some other issue and try to fix them.

[magrinj]

@Souravpakhira Thanks for the link, we're not handling the errors that way at the time and using the "old" way of GraphQL errors, by reading the errors data directly from the errors key inside the result. Apollo Client also handle errors that way on the front, so I'm not sure if it's a good option to move errors inside data. For the metadata errors, @Weiko has already add the exception-handler, the only missing part I guess is to change the error so it's not handled as a 500 🙂

[magrinj]

Closing as it's now fixed ! 🙂