Better diagnostics and/or default for MELI_MAX_ORGS

[mtiller]

I installed meli using in-memory auth. I created an org and everything worked great. Then I decided to implement our SSO system based on Gitlab. I eventually got that working as well. But when I logged in via Gitlab and tried to create an organization, I couldn't. The error message was just a 403 + could not create org. The "could not create org" is coming from the client side. But the server is actually returning "Cannot create more orgs" in its response. It seems the client just fails to read the message provided by the server and render it in the UI.

So I looked at the code and specifically the tests to see when a 403 would appear and it was because the limit on organizations had been reached. So then I looked for the default and found that it was just one. I changed my deployment to use a larger number and everything is fine.

So I have a few suggestions to avoid this confusion in the future. The first is that you are producing a response in the 403 that includes an error message so the UI should actually render that message. This provides the user with something actionable to address. I also wonder why the default maximum for organizations is one? It is true that I'll probably only use one, but it seems like multiple people might fall into this trap when switching authentication schemes because if I log in via "in memory" I see one set of organizations and if I log in via Gitlab I see an entirely different and mutually exclusive set of organizations. I can imagine other users getting caught in this if they decide to switch authentication methods and a) suddenly all their existing orgs disappear and b) they can't actually see (even if they are using the in-memory credentials which seems like a "root" account) all the organizations so they can't know whether they are exceeding the limit.

I'm curious, is there a reason why the default org limit is only 1? Without a diagnostic message that clearly states why the organization creation failed, I could see many users trying to create their second organization and scratching their head as to why that should fail when they managed to create a previous one just fine.

Anyway, just some suggestions to smooth the onboarding process for new users. Getting into the plumbing of how you client does error handling is a bit much for me to bite off with a PR to fix the error handling. But a simple solution might be to just raise the org limit so people are less likely to bump into this issue.

[gempain]

You're absolutely right, the current frontend error management lacks the ability to extract error messages from backend errors. The reason is we're doing toast.error(Could not create org: ${err}); which converts the error object to a string via a JS string template, only extracting the Error.message (native behavior of error.toString()) instead of actually extracting the message from the backend response. These errors are actually axios errors and the backend message can be retrieved from error.response.data.message. A safe option would be to do it this way: toast.error(Could not create org: ${error.response?.data?.message || error.message});. But I would extract this to a function to generalize the error handling behavior across the app.

As Meli is meant to be deployed on your VPS, 1 is a sane default, as we expect that you most likely not want your users to create more organizations.

[mtiller]

@gemapin It appears somebody already did "extract this to a function". It is called extractExrrorMessage and it can be found in ui/src/utils/extract-exrror-message. I thought the name of the file was a typo until I saw it matches the name of the function. Is this a typo? I don't see why the extra x is in there. But in any case, this appears to be the function to use to extract an error message generically. I can submit a pull request with all the toast calls invoking this function. But do you want me to rename this function while I'm at it?!?

[gempain]

Oh yes that's right, I think I created this function and left that aside for some reason. You can definitely reuse that and make the changes you suggested. You can also rename the function, it's a typo 😅 . I'd change the function however, because I realize that the response.data.message is potentially null, so to be safe here's what I'd do:

import { AxiosError } from 'axios';

export function extractErrorMessage(err: any) {
  let message: string;
  if (err.isAxiosError) {
    message = (err as AxiosError).response?.data?.message;
  }
  return message || err.message;
}