From f8c4ea62e47d78db5a7f45cc356f047151cb1cdc Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas <jvargas@gitlab.com> Date: Tue, 5 Mar 2024 00:32:09 +0000 Subject: [PATCH] Improve error handling in the GCP runner registration This adds a Toast message and changes how we log errors to Sentry as well --- ...google_cloud_registration_instructions.vue | 46 +++++++++++++++---- locale/gitlab.pot | 3 ++ ...le_cloud_registration_instructions_spec.js | 26 ++++++++++- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/ci/runner/components/registration/google_cloud_registration_instructions.vue b/app/assets/javascripts/ci/runner/components/registration/google_cloud_registration_instructions.vue index dadd1ba4e76b7..0bfc188001e3f 100644 --- a/app/assets/javascripts/ci/runner/components/registration/google_cloud_registration_instructions.vue +++ b/app/assets/javascripts/ci/runner/components/registration/google_cloud_registration_instructions.vue @@ -13,6 +13,7 @@ import { import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import { createAlert } from '~/alert'; import { s__, __ } from '~/locale'; +import { fetchPolicies } from '~/lib/graphql'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { TYPENAME_CI_RUNNER } from '~/graphql_shared/constants'; import runnerForRegistrationQuery from '../../graphql/register/runner_for_registration.query.graphql'; @@ -96,7 +97,12 @@ export default { ), }, copyCommands: __('Copy commands'), - alertBody: s__('Runners|To view the setup instructions, complete the previous form.'), + emptyFieldsAlertMessage: s__( + 'Runners|To view the setup instructions, complete the previous form.', + ), + invalidFieldsAlertMessage: s__( + 'Runners|To view the setup instructions, make sure all form fields are completed and correct.', + ), invalidFormButton: s__('Runners|Go to first invalid form field'), externalLink: __('(external link)'), }, @@ -197,6 +203,7 @@ export default { }, project: { query: provisionGoogleCloudRunnerProject, + fetchPolicy: fetchPolicies.NETWORK_ONLY, variables() { return { fullPath: this.projectPath, @@ -207,12 +214,15 @@ export default { runnerToken: this.token, }; }, - result({ data }) { - this.provisioningSteps = data.project.runnerCloudProvisioning?.provisioningSteps; - this.setupBashScript = data.project.runnerCloudProvisioning?.projectSetupShellScript; + result({ data, error }) { + if (!error) { + this.showAlert = false; + this.provisioningSteps = data.project.runnerCloudProvisioning?.provisioningSteps; + this.setupBashScript = data.project.runnerCloudProvisioning?.projectSetupShellScript; + } }, error(error) { - captureException({ error, component: this.$options.name }); + this.handleError(error); }, skip() { return !this.projectPath || this.invalidFields.length > 0; @@ -230,12 +240,15 @@ export default { runnerToken: this.token, }; }, - result({ data }) { - this.provisioningSteps = data.group.runnerCloudProvisioning?.provisioningSteps; - this.setupBashScript = data.group.runnerCloudProvisioning?.projectSetupShellScript; + result({ data, error }) { + if (!error) { + this.showAlert = false; + this.provisioningSteps = data.group.runnerCloudProvisioning?.provisioningSteps; + this.setupBashScript = data.group.runnerCloudProvisioning?.projectSetupShellScript; + } }, error(error) { - captureException({ error, component: this.$options.name }); + this.handleError(error); }, skip() { return !this.groupPath || this.invalidFields.length > 0; @@ -321,6 +334,14 @@ export default { this.$refs[this.invalidFields[0]].$el.focus(); } }, + handleError(error) { + if (error.message.includes('GraphQL error')) { + this.showAlert = true; + } + if (error.networkError) { + captureException({ error, component: this.$options.name }); + } + }, }, cancelModalOptions: { text: __('Close'), @@ -543,7 +564,12 @@ export default { class="gl-mb-4" @primaryAction="goToFirstInvalidField" > - {{ $options.i18n.alertBody }} + <template v-if="invalidFields.length > 0"> + {{ $options.i18n.emptyFieldsAlertMessage }} + </template> + <template v-else> + {{ $options.i18n.invalidFieldsAlertMessage }} + </template> </gl-alert> <gl-button data-testid="show-instructions-button" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 08d2f1dc6a421..de8f37aa7433d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -43518,6 +43518,9 @@ msgstr "" msgid "Runners|To view the setup instructions, complete the previous form. The instructions help you set up an autoscaling fleet of runners to execute your CI/CD jobs in Google Cloud." msgstr "" +msgid "Runners|To view the setup instructions, make sure all form fields are completed and correct." +msgstr "" + msgid "Runners|Token expiry" msgstr "" diff --git a/spec/frontend/ci/runner/components/registration/google_cloud_registration_instructions_spec.js b/spec/frontend/ci/runner/components/registration/google_cloud_registration_instructions_spec.js index 333e2e179b748..6b350f0cef3f6 100644 --- a/spec/frontend/ci/runner/components/registration/google_cloud_registration_instructions_spec.js +++ b/spec/frontend/ci/runner/components/registration/google_cloud_registration_instructions_spec.js @@ -89,6 +89,9 @@ describe('GoogleCloudRegistrationInstructions', () => { const projectInstructionsResolver = jest.fn().mockResolvedValue(mockProjectRunnerCloudSteps); const groupInstructionsResolver = jest.fn().mockResolvedValue(mockGroupRunnerCloudSteps); + const error = new Error('GraphQL error: One or more validations have failed'); + const errorResolver = jest.fn().mockRejectedValue(error); + const defaultHandlers = [[runnerForRegistrationQuery, runnerWithTokenResolver]]; const defaultProps = { runnerId: mockRunnerId, @@ -163,7 +166,7 @@ describe('GoogleCloudRegistrationInstructions', () => { expect(findClipboardButton().exists()).toBe(false); }); - it('Shows an alert when the form is not valid', async () => { + it('Shows an alert when the form has empty fields', async () => { createComponent(); findInstructionsButton().vm.$emit('click'); @@ -171,6 +174,7 @@ describe('GoogleCloudRegistrationInstructions', () => { await waitForPromises(); expect(findAlert().exists()).toBe(true); + expect(findAlert().text()).toBe('To view the setup instructions, complete the previous form.'); }); it('Hides an alert when the form is valid', async () => { @@ -221,4 +225,24 @@ describe('GoogleCloudRegistrationInstructions', () => { expect(findModalTerrarformInstructions().text()).not.toBeNull(); expect(findModalTerrarformApplyInstructions().text).not.toBeNull(); }); + + it('Does not display a modal with text when validation errors occur', async () => { + createComponent(mountExtended, [[provisionGoogleCloudRunnerQueryProject, errorResolver]]); + + findProjectIdInput().vm.$emit('input', 'dev-gcp-xxx-integrati-xxxxxxxx'); + findRegionInput().vm.$emit('input', 'us-central1xxx'); + findZoneInput().vm.$emit('input', 'us-central181263'); + + await waitForPromises(); + + expect(errorResolver).toHaveBeenCalled(); + + expect(findAlert().text()).toContain( + 'To view the setup instructions, make sure all form fields are completed and correct.', + ); + + expect(findModalBashInstructions().exists()).toBe(false); + expect(findModalTerrarformInstructions().exists()).toBe(false); + expect(findModalTerrarformApplyInstructions().exists()).toBe(false); + }); }); -- GitLab