From 38396b5882649d950beeea0e80b636e73cc8606f Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Wed, 30 Apr 2025 11:44:19 -0600 Subject: [PATCH] [UI] API Service Error Parsing (#30454) * adds error parsing method to api service * replaces apiErrorMessage util instances with api service parseError * removes apiErrorMessage util and tests * removes ApiError type * fixes issue in isLocalStorageSupported error handling --- ui/app/components/auth-form.js | 5 +- ui/app/components/control-group-success.js | 5 +- ui/app/components/tools/hash.ts | 4 +- ui/app/components/tools/lookup.ts | 6 +- ui/app/components/tools/random.ts | 4 +- ui/app/components/tools/rewrap.ts | 4 +- ui/app/components/tools/unwrap.ts | 4 +- ui/app/components/tools/wrap.ts | 4 +- ui/app/forms/README.md | 3 +- ui/app/lib/local-storage.ts | 9 ++- ui/app/services/api.ts | 57 ++++++++------ ui/app/utils/api-error-message.ts | 34 -------- .../messages/page/create-and-edit.js | 4 +- .../addon/components/messages/page/details.js | 5 +- .../addon/components/messages/page/list.js | 5 +- ui/tests/unit/services/api-test.js | 77 +++++++++++++++---- ui/tests/unit/utils/api-error-message-test.js | 62 --------------- ui/types/vault/api.d.ts | 15 ---- 18 files changed, 124 insertions(+), 183 deletions(-) delete mode 100644 ui/app/utils/api-error-message.ts delete mode 100644 ui/tests/unit/utils/api-error-message-test.js diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js index a2050ffa2e..dc53d85e8d 100644 --- a/ui/app/components/auth-form.js +++ b/ui/app/components/auth-form.js @@ -13,7 +13,6 @@ import { allSupportedAuthBackends, supportedAuthBackends } from 'vault/helpers/s import { task } from 'ember-concurrency'; import { waitFor } from '@ember/test-waiters'; import { v4 as uuidv4 } from 'uuid'; -import apiErrorMessage from 'vault/utils/api-error-message'; /** * @module AuthForm @@ -190,8 +189,8 @@ export default Component.extend(DEFAULTS, { this.set('token', response.auth.clientToken); this.send('doSubmit'); } catch (e) { - const error = yield apiErrorMessage(e); - this.set('error', `Token unwrap failed: ${error}`); + const { message } = yield this.api.parseError(e); + this.set('error', `Token unwrap failed: ${message}`); } }) ), diff --git a/ui/app/components/control-group-success.js b/ui/app/components/control-group-success.js index b731283e7a..a33c6de148 100644 --- a/ui/app/components/control-group-success.js +++ b/ui/app/components/control-group-success.js @@ -6,7 +6,6 @@ import { service } from '@ember/service'; import Component from '@ember/component'; import { task } from 'ember-concurrency'; -import apiErrorMessage from 'vault/utils/api-error-message'; export default Component.extend({ router: service(), @@ -28,8 +27,8 @@ export default Component.extend({ this.set('unwrapData', response.auth || response.data); this.controlGroup.deleteControlGroupToken(this.model.id); } catch (e) { - const error = yield apiErrorMessage(e); - this.error = `Token unwrap failed: ${error}`; + const { message } = yield this.api.parseError(e); + this.error = `Token unwrap failed: ${message}`; } }).drop(), diff --git a/ui/app/components/tools/hash.ts b/ui/app/components/tools/hash.ts index f8366475da..8ba110696c 100644 --- a/ui/app/components/tools/hash.ts +++ b/ui/app/components/tools/hash.ts @@ -7,7 +7,6 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; -import apiErrorMessage from 'vault/utils/api-error-message'; import type ApiService from 'vault/services/api'; import type FlashMessageService from 'vault/services/flash-messages'; @@ -60,7 +59,8 @@ export default class ToolsHash extends Component { this.sum = sum || ''; this.flashMessages.success('Hash was successful.'); } catch (error) { - this.errorMessage = await apiErrorMessage(error); + const { message } = await this.api.parseError(error); + this.errorMessage = message; } } } diff --git a/ui/app/components/tools/lookup.ts b/ui/app/components/tools/lookup.ts index 2d9e56ef16..0bde98b6e4 100644 --- a/ui/app/components/tools/lookup.ts +++ b/ui/app/components/tools/lookup.ts @@ -7,7 +7,6 @@ import Component from '@glimmer/component'; import { service } from '@ember/service'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; -import apiErrorMessage from 'vault/utils/api-error-message'; import { addSeconds } from 'date-fns'; import type ApiService from 'vault/services/api'; @@ -56,8 +55,9 @@ export default class ToolsLookup extends Component { const data = await this.api.sys.readWrappingProperties(payload); this.lookupData = data; this.flashMessages.success('Lookup was successful.'); - } catch (error) { - this.errorMessage = await apiErrorMessage(error); + } catch (e) { + const { message } = await this.api.parseError(e); + this.errorMessage = message; } } } diff --git a/ui/app/components/tools/random.ts b/ui/app/components/tools/random.ts index 98fb6ab723..960c157850 100644 --- a/ui/app/components/tools/random.ts +++ b/ui/app/components/tools/random.ts @@ -7,7 +7,6 @@ import Component from '@glimmer/component'; import { service } from '@ember/service'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; -import apiErrorMessage from 'vault/utils/api-error-message'; import type ApiService from 'vault/services/api'; import type FlashMessageService from 'vault/services/flash-messages'; @@ -52,7 +51,8 @@ export default class ToolsRandom extends Component { this.randomBytes = randomBytes || ''; this.flashMessages.success('Generated random bytes successfully.'); } catch (error) { - this.errorMessage = await apiErrorMessage(error); + const { message } = await this.api.parseError(error); + this.errorMessage = message; } } } diff --git a/ui/app/components/tools/rewrap.ts b/ui/app/components/tools/rewrap.ts index 6f8836db2a..7aebab95a6 100644 --- a/ui/app/components/tools/rewrap.ts +++ b/ui/app/components/tools/rewrap.ts @@ -7,7 +7,6 @@ import Component from '@glimmer/component'; import { service } from '@ember/service'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; -import apiErrorMessage from 'vault/utils/api-error-message'; import type ApiService from 'vault/services/api'; import type FlashMessageService from 'vault/services/flash-messages'; @@ -46,7 +45,8 @@ export default class ToolsRewrap extends Component { this.rewrappedToken = wrapInfo?.token || ''; this.flashMessages.success('Rewrap was successful.'); } catch (error) { - this.errorMessage = await apiErrorMessage(error); + const { message } = await this.api.parseError(error); + this.errorMessage = message; } } } diff --git a/ui/app/components/tools/unwrap.ts b/ui/app/components/tools/unwrap.ts index 4efca96bcc..b28e0b992c 100644 --- a/ui/app/components/tools/unwrap.ts +++ b/ui/app/components/tools/unwrap.ts @@ -7,7 +7,6 @@ import Component from '@glimmer/component'; import { service } from '@ember/service'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; -import apiErrorMessage from 'vault/utils/api-error-message'; import type ApiService from 'vault/services/api'; import type FlashMessageService from 'vault/services/flash-messages'; @@ -54,7 +53,8 @@ export default class ToolsUnwrap extends Component { }; this.flashMessages.success('Unwrap was successful.'); } catch (error) { - this.errorMessage = await apiErrorMessage(error); + const { message } = await this.api.parseError(error); + this.errorMessage = message; } } } diff --git a/ui/app/components/tools/wrap.ts b/ui/app/components/tools/wrap.ts index c7c30826ad..dc35e9ee60 100644 --- a/ui/app/components/tools/wrap.ts +++ b/ui/app/components/tools/wrap.ts @@ -8,7 +8,6 @@ import { service } from '@ember/service'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; import { stringify } from 'core/helpers/stringify'; -import apiErrorMessage from 'vault/utils/api-error-message'; import type ApiService from 'vault/services/api'; import type FlashMessageService from 'vault/services/flash-messages'; @@ -87,7 +86,8 @@ export default class ToolsWrap extends Component { this.token = wrapInfo?.token || ''; this.flashMessages.success('Wrap was successful.'); } catch (error) { - this.errorMessage = await apiErrorMessage(error); + const { message } = await this.api.parseError(error); + this.errorMessage = message; } } } diff --git a/ui/app/forms/README.md b/ui/app/forms/README.md index fb3b63cc4d..c1ca6bbe90 100644 --- a/ui/app/forms/README.md +++ b/ui/app/forms/README.md @@ -81,7 +81,8 @@ async save() { this.router.transitionTo('another.route'); } } catch(error) { - this.error = await apiErrorMessage(error); + const { message } = await this.api.parseError(error); + this.errorMessage = message; } } ``` diff --git a/ui/app/lib/local-storage.ts b/ui/app/lib/local-storage.ts index 8da3e057f5..6898d2d242 100644 --- a/ui/app/lib/local-storage.ts +++ b/ui/app/lib/local-storage.ts @@ -3,8 +3,6 @@ * SPDX-License-Identifier: BUSL-1.1 */ -import type { ApiError } from 'vault/api'; - export default { isLocalStorageSupported() { try { @@ -13,10 +11,13 @@ export default { window.localStorage.removeItem(key); return true; } catch (e) { - const error = e as ApiError; // modify the e object so we can customize the error message. // e.message is readOnly. - error.errors = [`This is likely due to your browser's cookie settings.`]; + Object.defineProperty(e, 'errors', { + value: [`This is likely due to your browser's cookie settings.`], + writable: false, + }); + throw e; } }, diff --git a/ui/app/services/api.ts b/ui/app/services/api.ts index c55351ab2e..67a6e43f34 100644 --- a/ui/app/services/api.ts +++ b/ui/app/services/api.ts @@ -15,15 +15,16 @@ import { HTTPQuery, HTTPRequestInit, RequestOpts, + ResponseError, } from '@hashicorp/vault-client-typescript'; -import config from '../config/environment'; +import config from 'vault/config/environment'; import { waitForPromise } from '@ember/test-waiters'; import type AuthService from 'vault/services/auth'; import type NamespaceService from 'vault/services/namespace'; import type ControlGroupService from 'vault/services/control-group'; import type FlashMessageService from 'vault/services/flash-messages'; -import type { ApiError, HeaderMap, XVaultHeaders } from 'vault/api'; +import type { HeaderMap, XVaultHeaders } from 'vault/api'; export default class ApiService extends Service { @service('auth') declare readonly authService: AuthService; @@ -103,27 +104,6 @@ export default class ApiService extends Service { this.controlGroup.deleteControlGroupToken(controlGroupToken.accessor); } }; - - formatErrorResponse = async (context: ResponseContext) => { - const response = context.response.clone(); - const { headers, status, statusText } = response; - - // backwards compatibility with Ember Data - if (status >= 400) { - const error: ApiError = (await response?.json()) || {}; - error.httpStatus = response?.status; - error.path = context.url; - // typically the Vault API error response looks like { errors: ['some error message'] } - // but sometimes (eg RespondWithStatusCode) it's { data: { error: 'some error message' } } - if (error?.data?.error && !error.errors) { - // normalize the errors from RespondWithStatusCode - error.errors = [error.data.error]; - } - return new Response(JSON.stringify(error), { headers, status, statusText }); - } - - return; - }; // --- End Middleware --- configuration = new Configuration({ @@ -134,7 +114,6 @@ export default class ApiService extends Service { { pre: this.setHeaders }, { post: this.showWarnings }, { post: this.deleteControlGroupToken }, - { post: this.formatErrorResponse }, ], fetchApi: (...args: [Request]) => { return waitForPromise(window.fetch(...args)); @@ -172,4 +151,34 @@ export default class ApiService extends Service { const { context } = requestContext; context.query = { ...context.query, ...params }; } + + // accepts an error response and returns { status, message, response, path } + // message is built as error.errors joined with a comma, error.message or a fallback message + // path is the url of the request, minus the origin -> /v1/sys/wrapping/unwrap + async parseError(e: unknown, fallbackMessage = 'An error occurred, please try again') { + if (e instanceof ResponseError) { + const { status, url } = e.response; + const error = await e.response.json(); + // typically the Vault API error response looks like { errors: ['some error message'] } + // but sometimes (eg RespondWithStatusCode) it's { data: { error: 'some error message' } } + const errors = error.data?.error && !error.errors ? [error.data.error] : error.errors; + const message = errors && typeof errors[0] === 'string' ? errors.join(', ') : error.message; + + return { + message: message || fallbackMessage, + status, + path: url.replace(document.location.origin, ''), + response: error, + }; + } + + // log out generic error for ease of debugging in dev env + if (config.environment === 'development') { + console.log('API Error:', e); // eslint-disable-line no-console + } + + return { + message: (e as Error)?.message || fallbackMessage, + }; + } } diff --git a/ui/app/utils/api-error-message.ts b/ui/app/utils/api-error-message.ts deleted file mode 100644 index dd603a764a..0000000000 --- a/ui/app/utils/api-error-message.ts +++ /dev/null @@ -1,34 +0,0 @@ -/** - * Copyright (c) HashiCorp, Inc. - * SPDX-License-Identifier: BUSL-1.1 - */ - -/** - * this util was derived from error-message and updated to handle the error context returned from the api service - * once Ember Data is fully removed, the error-message util will also be removed - * for all requests made with the api service, use this util to display error messages from server - */ - -import { ErrorContext, ApiError } from 'vault/api'; -import ENV from 'vault/config/environment'; - -// accepts an error and returns error.errors joined with a comma, error.message or a fallback message -export default async function (error: unknown, fallbackMessage = 'An error occurred, please try again') { - const messageOrFallback = (message?: string) => message || fallbackMessage; - - // log out the error for ease of debugging in dev env - if (ENV.environment === 'development') { - console.error('API Error:', error); // eslint-disable-line no-console - } - - if ((error as ErrorContext).response instanceof Response) { - const apiError: ApiError = await (error as ErrorContext).response?.json(); - - if (apiError.errors && typeof apiError.errors[0] === 'string') { - return apiError.errors.join(', '); - } - return messageOrFallback(apiError.message); - } - - return messageOrFallback((error as Error)?.message); -} diff --git a/ui/lib/config-ui/addon/components/messages/page/create-and-edit.js b/ui/lib/config-ui/addon/components/messages/page/create-and-edit.js index 4f75abf265..a834401899 100644 --- a/ui/lib/config-ui/addon/components/messages/page/create-and-edit.js +++ b/ui/lib/config-ui/addon/components/messages/page/create-and-edit.js @@ -6,7 +6,6 @@ import Component from '@glimmer/component'; import { tracked } from '@glimmer/tracking'; import { task, timeout } from 'ember-concurrency'; -import apiErrorMessage from 'vault/utils/api-error-message'; import { service } from '@ember/service'; import { action } from '@ember/object'; import Ember from 'ember'; @@ -90,7 +89,8 @@ export default class MessagesList extends Component { this.router.transitionTo('vault.cluster.config-ui.messages.message.details', id); } } catch (error) { - this.errorBanner = yield apiErrorMessage(error); + const { message } = yield this.api.parseError(error); + this.errorBanner = message; this.invalidFormAlert = 'There was an error submitting this form.'; } } diff --git a/ui/lib/config-ui/addon/components/messages/page/details.js b/ui/lib/config-ui/addon/components/messages/page/details.js index 3609d8dc46..d706ecb9b1 100644 --- a/ui/lib/config-ui/addon/components/messages/page/details.js +++ b/ui/lib/config-ui/addon/components/messages/page/details.js @@ -6,7 +6,6 @@ import Component from '@glimmer/component'; import { service } from '@ember/service'; import { action } from '@ember/object'; -import apiErrorMessage from 'vault/utils/api-error-message'; /** * @module Page::MessageDetails @@ -37,8 +36,8 @@ export default class MessageDetails extends Component { this.customMessages.fetchMessages(); this.flashMessages.success(`Successfully deleted ${message.title}.`); } catch (e) { - const errorMessage = await apiErrorMessage(e); - this.flashMessages.danger(errorMessage); + const { message } = await this.api.parseError(e); + this.flashMessages.danger(message); } } } diff --git a/ui/lib/config-ui/addon/components/messages/page/list.js b/ui/lib/config-ui/addon/components/messages/page/list.js index eb039c5f00..6dbd7852be 100644 --- a/ui/lib/config-ui/addon/components/messages/page/list.js +++ b/ui/lib/config-ui/addon/components/messages/page/list.js @@ -10,7 +10,6 @@ import { task, timeout } from 'ember-concurrency'; import { dateFormat } from 'core/helpers/date-format'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; -import apiErrorMessage from 'vault/utils/api-error-message'; import { isAfter } from 'date-fns'; import timestamp from 'core/utils/timestamp'; @@ -102,8 +101,8 @@ export default class MessagesList extends Component { this.customMessages.fetchMessages(); this.flashMessages.success(`Successfully deleted ${message.title}.`); } catch (e) { - const errorMessage = yield apiErrorMessage(e); - this.flashMessages.danger(errorMessage); + const { message } = yield this.api.parseError(e); + this.flashMessages.danger(message); } finally { this.messageToDelete = null; } diff --git a/ui/tests/unit/services/api-test.js b/ui/tests/unit/services/api-test.js index 0d92d238f3..8dadb7e7c7 100644 --- a/ui/tests/unit/services/api-test.js +++ b/ui/tests/unit/services/api-test.js @@ -6,6 +6,8 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; import sinon from 'sinon'; +import config from 'vault/config/environment'; +import { ResponseError } from '@hashicorp/vault-client-typescript'; module('Unit | Service | api', function (hooks) { setupTest(hooks); @@ -136,22 +138,6 @@ module('Unit | Service | api', function (hooks) { ); }); - test('it should format error response', async function (assert) { - const e = { data: { error: 'Something went wrong' } }; - const response = new Response(JSON.stringify(e), { status: 400 }); - - const errorResponse = await this.apiService.formatErrorResponse({ response, url: this.url }); - const error = await errorResponse.json(); - const expectedError = { - ...e, - httpStatus: 400, - path: this.url, - errors: ['Something went wrong'], - }; - - assert.deepEqual(error, expectedError, 'Error is reformated and returned'); - }); - test('it should build headers', async function (assert) { const headerMap = { token: 'foobar', @@ -183,4 +169,63 @@ module('Unit | Service | api', function (hooks) { 'All supported headers are set' ); }); + + module('Error parsing', function (hooks) { + hooks.beforeEach(function () { + this.response = { + errors: ['first error', 'second error'], + message: 'there were some errors', + }; + this.getErrorResponse = () => + new ResponseError({ + status: 404, + url: `${document.location.origin}/v1/test/error/parsing`, + json: () => Promise.resolve(this.response), + }); + }); + + test('it should correctly parse message from error', async function (assert) { + let e = await this.apiService.parseError(this.getErrorResponse()); + assert.strictEqual(e.message, 'first error, second error', 'Builds message from errors'); + + this.response.errors = []; + e = await this.apiService.parseError(this.getErrorResponse()); + assert.strictEqual(e.message, 'there were some errors', 'Returns message when errors are empty'); + + const error = new Error('some js type error'); + e = await this.apiService.parseError(error); + assert.strictEqual(e.message, error.message, 'Returns message from generic Error'); + + e = await this.apiService.parseError('some random error'); + assert.strictEqual(e.message, 'An error occurred, please try again', 'Returns default fallback'); + + const fallback = 'Everything is broken, sorry'; + e = await this.apiService.parseError('some random error', fallback); + assert.strictEqual(e.message, fallback, 'Returns custom fallback'); + }); + + test('it should return status', async function (assert) { + const { status } = await this.apiService.parseError(this.getErrorResponse()); + assert.strictEqual(status, 404, 'Returns the status code from the response'); + }); + + test('it should return path', async function (assert) { + const { path } = await this.apiService.parseError(this.getErrorResponse()); + assert.strictEqual(path, '/v1/test/error/parsing', 'Returns the path from the request url'); + }); + + test('it should return error response', async function (assert) { + const { response } = await this.apiService.parseError(this.getErrorResponse()); + assert.deepEqual(response, this.response, 'Returns the original error response'); + }); + + test('it should log out error in development environment', async function (assert) { + const consoleStub = sinon.stub(console, 'log'); + sinon.stub(config, 'environment').value('development'); + const error = new Error('some js type error'); + await this.apiService.parseError(error); + assert.true(consoleStub.calledWith('API Error:', error)); + sinon.restore(); + }); + }); }); diff --git a/ui/tests/unit/utils/api-error-message-test.js b/ui/tests/unit/utils/api-error-message-test.js deleted file mode 100644 index ad9f5d66b2..0000000000 --- a/ui/tests/unit/utils/api-error-message-test.js +++ /dev/null @@ -1,62 +0,0 @@ -/** - * Copyright (c) HashiCorp, Inc. - * SPDX-License-Identifier: BUSL-1.1 - */ - -import { module, test } from 'qunit'; -import apiErrorMessage from 'vault/utils/api-error-message'; -import ENV from 'vault/config/environment'; -import sinon from 'sinon'; - -module('Unit | Util | api-error-message', function (hooks) { - hooks.beforeEach(function () { - this.apiError = { - errors: ['first error', 'second error'], - message: 'there were some errors', - }; - this.getErrorContext = () => ({ response: new Response(JSON.stringify(this.apiError)) }); - }); - - test('it should return errors from ErrorContext', async function (assert) { - const message = await apiErrorMessage(this.getErrorContext()); - assert.strictEqual(message, 'first error, second error'); - }); - - test('it should return message from ErrorContext when errors are empty', async function (assert) { - this.apiError.errors = []; - const message = await apiErrorMessage(this.getErrorContext()); - assert.strictEqual(message, 'there were some errors'); - }); - - test('it should return fallback message for ErrorContext without errors or message', async function (assert) { - this.apiError = {}; - const message = await apiErrorMessage(this.getErrorContext()); - assert.strictEqual(message, 'An error occurred, please try again'); - }); - - test('it should return message from Error', async function (assert) { - const error = new Error('some js type error'); - const message = await apiErrorMessage(error); - assert.strictEqual(message, error.message); - }); - - test('it should return default fallback', async function (assert) { - const message = await apiErrorMessage('some random error'); - assert.strictEqual(message, 'An error occurred, please try again'); - }); - - test('it should return custom fallback message', async function (assert) { - const fallback = 'Everything is broken, sorry'; - const message = await apiErrorMessage('some random error', fallback); - assert.strictEqual(message, fallback); - }); - - test('it should log out error in development environment', async function (assert) { - const consoleStub = sinon.stub(console, 'error'); - sinon.stub(ENV, 'environment').value('development'); - const error = new Error('some js type error'); - await apiErrorMessage(error); - assert.true(consoleStub.calledWith('API Error:', error)); - sinon.restore(); - }); -}); diff --git a/ui/types/vault/api.d.ts b/ui/types/vault/api.d.ts index 0df1d919e2..d5247c6b87 100644 --- a/ui/types/vault/api.d.ts +++ b/ui/types/vault/api.d.ts @@ -3,21 +3,6 @@ * SPDX-License-Identifier: BUSL-1.1 */ -import { ErrorContext } from '@hashicorp/vault-client-typescript'; - -// re-exporting for convenience since it is associated to ApiError -export { ErrorContext }; -export interface ApiError { - httpStatus: number; - path: string; - message: string; - errors: Array; - data?: { - [key: string]: unknown; - error?: string; - }; -} - export interface WrapInfo { accessor: string; creation_path: string;