chore: improve api error messages (CM-855) #3713
Conversation
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
2 similar comments
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
joanagmaia
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (data.name || data.slug) { | ||
| await this.validateUpdateDuplicates(id, segment, data, segmentRepository) | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: Update validation misses slug conflicts from isLF transformation
When updating a segment, validateUpdateDuplicates skips slug validation if data.slug === segment.slug (line 735). However, the slug is subsequently transformed by validateNonLfSlug when isLF changes to false (line 54). The transformed slug (with nonlf_ prefix) is never validated for conflicts. This allows creating duplicate slugs when a user updates a segment with the same slug value but changes isLF from true to false, potentially colliding with an existing segment that already has the nonlf_-prefixed version of that slug.
Additional Locations (1)
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
| 'Trying to update repo (?<repo>[^\\s]+) mapping with integrationId (?<IId>[^\\s]+) ' | ||
| + 'but it is already mapped to integration (?<eId>[^\\s!]+)', | ||
| ); | ||
| const match = errorMessage.match(pattern); |
There was a problem hiding this comment.
parseDuplicateRepoError crashes on non-string input
The parseDuplicateRepoError function calls errorMessage.match(pattern) without checking if errorMessage is a string. The callers pass error?.response?.data which can be undefined (on network errors or timeouts), null, or an object (if the API returns JSON). When errorMessage is not a string, calling .match() throws a TypeError, which will crash the error handler and prevent proper error display. The function needs to validate that errorMessage is a string before calling .match().
Additional Locations (2)
| const remotes = integrationData.remote.repoNames.map((repoName) => { | ||
| const fullUrl = stripGit(`${integrationData.remote.orgURL}/${repoName}`) | ||
| return { url: fullUrl, forkedFrom: null } | ||
| }) |
There was a problem hiding this comment.
Gerrit remotes built before enableAllRepos updates repoNames
The remotes array is built from integrationData.remote.repoNames before getGerritServerRepos is called. When enableAllRepos is true, integrationData.remote.repoNames gets updated with the full server list at line 1767, but remotes still contains the initial (possibly empty or partial) list. This causes two problems: the conflict check (lines 1729-1763) only validates the initial repos, and gitConnectOrUpdate (line 1806) only syncs the initial repos, while the integration settings store the complete list. This mismatch means repositories could be added without proper conflict checking and won't be synced to git.
Additional Locations (1)
| `SELECT id, settings FROM integrations | ||
| WHERE platform = 'gerrit' AND "deletedAt" IS NULL AND id != :currentIntegrationId`, | ||
| { | ||
| replacements: { currentIntegrationId: integration?.id || connectionId }, |
There was a problem hiding this comment.
Gerrit conflict check uses undefined currentIntegrationId
The Gerrit conflict check at line 1735 uses integration?.id || connectionId as currentIntegrationId, but both variables are undefined at this point in execution (they're assigned at lines 1770 and 1782, respectively). This causes two problems: the SQL query id != :currentIntegrationId with undefined doesn't exclude any rows (SQL id != NULL evaluates to NULL, not true), so for updates the check could incorrectly flag the integration's own repos as conflicts. Additionally, the error message at line 1757 would display undefined as the integration ID.
Changes
Repositories already mapped to other projects
Trying to update gitlab repo ${row.url} mapping with integrationId ${integrationId} but it is already mapped to integration ${row.integrationId}!Creating segments with names or slugs that already exist
Sub-project with slug "{0}" already exists in project {1}.,Project with slug "{0}" already exists in project group {1}.,Project Group with slug "{0}" already exists.,Sub-project {0} already exists in project {1}.,Project {0} already exists in project group {1}.,Project Group {0} already exists.Note
Strengthens validation and user-facing errors for code integrations and segments.
errors.integrations.repoAlreadyMapped; adds pre-checks to prevent mapping repos already linked to other integrations ingitConnectOrUpdate, GitLab repo mapping, and Gerrit connect (including URL normalization)findByNamein repository and centralized checks inSegmentServiceto block conflictingname/slugacross PROJECT_GROUP/PROJECT/SUB_PROJECT, with precise i18n messages and correct parent context; applied on create and updateerror-message.helperwithparseDuplicateRepoError,getAxiosErrorMessage, andcustomRepoErrorMessage; integrates consistent error handling in Git, Gerrit, and both GitHub settings drawers; propagateserrorHandlervia store actions; improves admin project/segment toastsWritten by Cursor Bugbot for commit 209bf1a. This will update automatically on new commits. Configure here.