-
Notifications
You must be signed in to change notification settings - Fork 728
chore: improve error messages #3716
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
base: chore/improve-api-error-messages
Are you sure you want to change the base?
chore: improve error messages #3716
Conversation
…rojects Signed-off-by: Efren Lim <elim@linuxfoundation.org>
…rojects Signed-off-by: Efren Lim <elim@linuxfoundation.org>
…/crowd.dev into chore/improve-error-messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conventional Commits FTW!
|
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. |
3 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. |
|
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. |
| '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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling .match() on potentially non-string value crashes handler
The errorHandler function retrieves error?.response?.data into errorMessage and immediately calls .match() on it. If error.response.data is undefined (network error) or an object (JSON error response), calling .match() will throw a TypeError, causing the error handler itself to crash. The codebase follows a pattern of checking typeof error?.response?.data === 'string' before using string methods (as seen in groupsio-settings-drawer.vue and the new getAxiosErrorMessage helper), but this check is missing here.
| href: getSegmentLink(segment), | ||
| class: 'text-blue-500 underline hover:text-blue-600', | ||
| }, | ||
| segment.name || 'Unknown Project', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing properties of potentially undefined segment object
The customErrorMessage function accesses segment.grandparentId, segment.id, and segment.name without checking if segment is defined. When IntegrationService.find(eId) returns an integration where integration.segment is null or undefined, passing it to customErrorMessage will cause a TypeError. The TODO comment on line 190 acknowledges potential data inconsistencies, making this edge case more likely to occur.
Additional Locations (1)
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
|
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. |
| '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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling match on undefined errorMessage causes crash
The parseDuplicateRepoError function calls errorMessage.match(pattern) without checking if errorMessage is defined. All callers pass error?.response?.data which can be undefined (e.g., network errors, timeouts, or errors without a response body). When errorMessage is undefined, calling .match() throws a TypeError, crashing the error handler instead of showing the default error message to users.
In this PR:
Ticket
CM-561
Note
Centralizes error handling and improves duplicate repository conflict messaging across integrations and project management flows.
shared/helpers/error-message.helper.tswithgetAxiosErrorMessage,parseDuplicateRepoError, andcustomRepoErrorMessageto centralize Axios error parsing and duplicate repo conflict messaging.git/gerrit/githubsettings drawers to use a unifiederrorHandlerleveragingparseDuplicateRepoErrorandIntegrationService.findto show contextual conflict toasts; remove ad-hoc parsing.integration-storeactionsdoGitConnectanddoGerritConnectto accept and invoke optionalerrorHandler.getAxiosErrorMessageinlf-project-group-form.vueand segmentsactions.jsto surface backend-provided messages for project group/project/sub-project create/update errors.Written by Cursor Bugbot for commit 019eac6. This will update automatically on new commits. Configure here.