From 364765cca03d40a4e1d4c130d6ecba04a81dba98 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 22 Nov 2023 14:16:03 -0500 Subject: [PATCH] remove response error from template and keep parsing at the client level Signed-off-by: Aramis Sennyey --- packages/catalog-client/src/CatalogClient.ts | 268 +++++++++--------- .../src/generated/apis/DefaultApi.client.ts | 106 +------ .../typescript-backstage/api.mustache | 8 +- 3 files changed, 152 insertions(+), 230 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index dd22a44d59..b1a50b5f07 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -39,24 +39,10 @@ import { GetEntitiesByRefsRequest, GetEntitiesByRefsResponse, QueryEntitiesRequest, - QueryEntitiesResponse, EntityFilterQuery, } from './types/api'; -import { DefaultApiClient } from './generated/apis/DefaultApi.client'; import { isQueryEntitiesInitialRequest } from './utils'; -import { ValidateEntity400Response } from './generated/models/ValidateEntity400Response.model'; - -const isResponseError = (e: Error): e is ResponseError => { - return (e as any).response; -}; - -const ignore404 = (responseError: Error) => { - if (!isResponseError(responseError)) throw responseError; - if (responseError.response.status === 404) { - return undefined; - } - throw responseError; -}; +import { DefaultApiClient } from './generated'; /** * A frontend and backend compatible client for communicating with the Backstage @@ -81,10 +67,12 @@ export class CatalogClient implements CatalogApi { request: GetEntityAncestorsRequest, options?: CatalogRequestOptions, ): Promise { - const { kind, namespace, name } = parseEntityRef(request.entityRef); - return await this.apiClient - .getEntityAncestryByName({ path: { kind, namespace, name } }, options) - .then(r => r.json()); + return await this.requestRequired( + await this.apiClient.getEntityAncestryByName( + { path: parseEntityRef(request.entityRef) }, + options, + ), + ); } /** @@ -94,13 +82,9 @@ export class CatalogClient implements CatalogApi { id: string, options?: CatalogRequestOptions, ): Promise { - try { - return await this.apiClient - .getLocation({ path: { id } }, options) - .then(r => r.json()); - } catch (e) { - return ignore404(e); - } + return await this.requestOptional( + await this.apiClient.getLocation({ path: { id } }, options), + ); } /** @@ -131,21 +115,21 @@ export class CatalogClient implements CatalogApi { } } - const entities: Entity[] = await this.apiClient - .getEntities( + const entities: Entity[] = await this.requestRequired( + await this.apiClient.getEntities( { query: { - fields: fields.map(encodeURIComponent), limit, - filter: this.getFilterValue(filter), offset, after, + filter: this.getFilterValue(filter), + fields, order: order ? encodedOrder : undefined, }, }, options, - ) - .then(r => r.json()); + ), + ); const refCompare = (a: Entity, b: Entity) => { // in case field filtering is used, these fields might not be part of the response @@ -179,9 +163,23 @@ export class CatalogClient implements CatalogApi { request: GetEntitiesByRefsRequest, options?: CatalogRequestOptions, ): Promise { - const { items } = await this.apiClient - .getEntitiesByRefs({ body: request }, options) - .then(r => r.json()); + const response = await this.apiClient.getEntitiesByRefs( + { + body: { + entityRefs: request.entityRefs, + fields: request.fields?.length ? request.fields : undefined, + }, + }, + options, + ); + + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + const { items } = (await response.json()) as { + items: Array; + }; return { items: items.map(i => i ?? undefined) }; } @@ -192,7 +190,7 @@ export class CatalogClient implements CatalogApi { async queryEntities( request: QueryEntitiesRequest = {}, options?: CatalogRequestOptions, - ): Promise { + ) { const params: Partial< Parameters[0]['query'] > = {}; @@ -238,7 +236,7 @@ export class CatalogClient implements CatalogApi { } } - return await this.apiClient + return this.apiClient .getEntitiesByQuery({ query: params }, options) .then(r => r.json()); } @@ -250,13 +248,14 @@ export class CatalogClient implements CatalogApi { entityRef: string | CompoundEntityRef, options?: CatalogRequestOptions, ): Promise { - try { - return await this.apiClient - .getEntityByName({ path: { ...parseEntityRef(entityRef) } }, options) - .then(r => r.json()); - } catch (e) { - return ignore404(e); - } + return this.requestOptional( + await this.apiClient.getEntityByName( + { + path: parseEntityRef(entityRef), + }, + options, + ), + ); } // NOTE(freben): When we deprecate getEntityByName from the interface, we may @@ -271,36 +270,26 @@ export class CatalogClient implements CatalogApi { options?: CatalogRequestOptions, ): Promise { const { kind, namespace = 'default', name } = compoundName; - try { - return await this.apiClient - .getEntityByName( - { - path: { - kind, - namespace, - name, - }, - }, - options, - ) - .then(r => r.json()); - } catch (e) { - return ignore404(e); - } + return this.requestOptional( + await this.apiClient.getEntityByName( + { path: { kind, namespace, name } }, + options, + ), + ); } /** * {@inheritdoc CatalogApi.refreshEntity} */ async refreshEntity(entityRef: string, options?: CatalogRequestOptions) { - await this.apiClient.refreshEntity( - { - body: { - entityRef, - }, - }, + const response = await this.apiClient.refreshEntity( + { body: { entityRef } }, options, ); + + if (response.status !== 200) { + throw new Error(await response.text()); + } } /** @@ -311,22 +300,14 @@ export class CatalogClient implements CatalogApi { options?: CatalogRequestOptions, ): Promise { const { filter = [], facets } = request; - - try { - return await this.apiClient - .getEntityFacets( - { - query: { - facet: facets, - filter: this.getFilterValue(filter), - }, - }, - options, - ) - .then(r => r.json()); - } catch (e) { - return ignore404(e) as any; - } + return await this.requestOptional( + await this.apiClient.getEntityFacets( + { + query: { facet: facets, filter: this.getFilterValue(filter) }, + }, + options, + ), + ); } /** @@ -338,18 +319,19 @@ export class CatalogClient implements CatalogApi { ): Promise { const { type = 'url', target, dryRun } = request; - const { location, entities, exists } = await this.apiClient - .createLocation( - { - body: { - type, - target, - }, - query: { dryRun: dryRun ? 'true' : undefined }, - }, - options, - ) - .then(r => r.json()); + const response = await this.apiClient.createLocation( + { + body: { type, target }, + query: { dryRun: dryRun ? 'true' : undefined }, + }, + options, + ); + + if (response.status !== 201) { + throw new Error(await response.text()); + } + + const { location, entities, exists } = await response.json(); if (!location) { throw new Error(`Location wasn't added: ${target}`); @@ -369,9 +351,9 @@ export class CatalogClient implements CatalogApi { locationRef: string, options?: CatalogRequestOptions, ): Promise { - const all: { data: Location }[] = await this.apiClient - .getLocations({}, options) - .then(r => r.json()); + const all: { data: Location }[] = await this.requestRequired( + await this.apiClient.getLocation({ path: { id: locationRef } }, options), + ); return all .map(r => r.data) .find(l => locationRef === stringifyLocationRef(l)); @@ -384,11 +366,9 @@ export class CatalogClient implements CatalogApi { id: string, options?: CatalogRequestOptions, ): Promise { - try { - await this.apiClient.deleteLocation({ path: { id } }, options); - } catch (e) { - ignore404(e); - } + await this.requestIgnored( + await this.apiClient.deleteLocation({ path: { id } }, options), + ); } /** @@ -398,11 +378,9 @@ export class CatalogClient implements CatalogApi { uid: string, options?: CatalogRequestOptions, ): Promise { - try { - await this.apiClient.deleteEntityByUid({ path: { uid } }, options); - } catch (e) { - ignore404(e); - } + await this.requestIgnored( + await this.apiClient.deleteEntityByUid({ path: { uid } }, options), + ); } /** @@ -413,38 +391,64 @@ export class CatalogClient implements CatalogApi { locationRef: string, options?: CatalogRequestOptions, ): Promise { - try { - await this.apiClient.validateEntity( - { - body: { - entity, - location: locationRef, - }, - }, - options, - ); + const response = await this.apiClient.validateEntity( + { body: { entity, location: locationRef } }, + options, + ); + if (response.ok) { return { valid: true, }; - } catch (e) { - if (!isResponseError(e)) throw e; - const { response, rawBody } = e; - if (response.status !== 400) { - throw e; - } - // TODO: This needs to be fixed as is, this won't actually do anything. - const { errors = [] } = JSON.parse(rawBody) as ValidateEntity400Response; + } - return { - valid: false, - errors, - }; + if (response.status !== 400) { + throw await ResponseError.fromResponse(response); + } + + const { errors = [] } = (await response.json()) as any; + + return { + valid: false, + errors, + }; + } + + // + // Private methods + // + + private async requestIgnored(response: Response): Promise { + if (!response.ok) { + throw await ResponseError.fromResponse(response); } } + private async requestRequired(response: Response): Promise { + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + return response.json(); + } + + private async requestOptional(response: Response): Promise { + if (!response.ok) { + if (response.status === 404) { + return undefined; + } + throw await ResponseError.fromResponse(response); + } + + return await response.json(); + } + private getFilterValue(filter: EntityFilterQuery = []) { - const values: string[] = []; + const filters: string[] = []; + // filter param can occur multiple times, for example + // /api/catalog/entities?filter=metadata.name=wayback-search,kind=component&filter=metadata.name=www-artist,kind=component' + // the "outer array" defined by `filter` occurrences corresponds to "anyOf" filters + // the "inner array" defined within a `filter` param corresponds to "allOf" filters for (const filterItem of [filter].flat()) { const filterParts: string[] = []; for (const [key, value] of Object.entries(filterItem)) { @@ -460,9 +464,9 @@ export class CatalogClient implements CatalogApi { } if (filterParts.length) { - values.push(filterParts.join(',')); + filters.push(filterParts.join(',')); } } - return values; + return filters; } } diff --git a/packages/catalog-client/src/generated/apis/DefaultApi.client.ts b/packages/catalog-client/src/generated/apis/DefaultApi.client.ts index 11a0b33bc8..260f7c9fc8 100644 --- a/packages/catalog-client/src/generated/apis/DefaultApi.client.ts +++ b/packages/catalog-client/src/generated/apis/DefaultApi.client.ts @@ -19,7 +19,6 @@ import { FetchApi } from '../types/fetch'; import crossFetch from 'cross-fetch'; import { pluginId } from '../pluginId'; import * as parser from 'uri-template'; -import { ResponseError } from '@backstage/errors'; import { AnalyzeLocationRequest } from '../models/AnalyzeLocationRequest.model'; import { AnalyzeLocationResponse } from '../models/AnalyzeLocationResponse.model'; @@ -81,8 +80,7 @@ export class DefaultApiClient { const uri = parser.parse(uriTemplate).expand({}); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), @@ -90,10 +88,6 @@ export class DefaultApiClient { method: 'POST', body: JSON.stringify(request.body), }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -119,8 +113,7 @@ export class DefaultApiClient { ...request.query, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), @@ -128,10 +121,6 @@ export class DefaultApiClient { method: 'POST', body: JSON.stringify(request.body), }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -155,18 +144,13 @@ export class DefaultApiClient { uid: request.path.uid, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'DELETE', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -190,18 +174,13 @@ export class DefaultApiClient { id: request.path.id, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'DELETE', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -235,18 +214,13 @@ export class DefaultApiClient { ...request.query, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -282,18 +256,13 @@ export class DefaultApiClient { ...request.query, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -313,8 +282,7 @@ export class DefaultApiClient { const uri = parser.parse(uriTemplate).expand({}); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), @@ -322,10 +290,6 @@ export class DefaultApiClient { method: 'POST', body: JSON.stringify(request.body), }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -355,18 +319,13 @@ export class DefaultApiClient { name: request.path.name, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -396,18 +355,13 @@ export class DefaultApiClient { name: request.path.name, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -431,18 +385,13 @@ export class DefaultApiClient { uid: request.path.uid, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -468,18 +417,13 @@ export class DefaultApiClient { ...request.query, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -503,18 +447,13 @@ export class DefaultApiClient { id: request.path.id, }); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -531,18 +470,13 @@ export class DefaultApiClient { const uri = parser.parse(uriTemplate).expand({}); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), }, method: 'GET', }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -562,8 +496,7 @@ export class DefaultApiClient { const uri = parser.parse(uriTemplate).expand({}); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), @@ -571,10 +504,6 @@ export class DefaultApiClient { method: 'POST', body: JSON.stringify(request.body), }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } /** @@ -594,8 +523,7 @@ export class DefaultApiClient { const uri = parser.parse(uriTemplate).expand({}); - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { 'Content-Type': 'application/json', ...(options?.token && { Authorization: `Bearer ${options?.token}` }), @@ -603,9 +531,5 @@ export class DefaultApiClient { method: 'POST', body: JSON.stringify(request.body), }); - if (response.ok) { - return response; - } - throw await ResponseError.fromResponse(response); } } diff --git a/packages/repo-tools/templates/typescript-backstage/api.mustache b/packages/repo-tools/templates/typescript-backstage/api.mustache index e2e1ae4b3b..1b8739ba57 100644 --- a/packages/repo-tools/templates/typescript-backstage/api.mustache +++ b/packages/repo-tools/templates/typescript-backstage/api.mustache @@ -4,7 +4,6 @@ import { FetchApi } from '../types/fetch'; import crossFetch from 'cross-fetch'; import {pluginId} from '../pluginId'; import * as parser from 'uri-template'; -import {ResponseError} from '@backstage/errors'; {{#imports}} import { {{classname}} } from '{{filename}}.model{{importFileExtension}}'; @@ -97,8 +96,7 @@ export class {{classname}}Client { {{/hasQueryParams}} }) - const url = `${baseUrl}${uri}`; - const response = await this.fetchApi.fetch(url, { + return await this.fetchApi.fetch(`${baseUrl}${uri}`, { headers: { {{#hasHeaderParams}} ...request.header, @@ -109,10 +107,6 @@ export class {{classname}}Client { method: '{{httpMethod}}', {{#hasBodyParam}} body: JSON.stringify(request.body), {{/hasBodyParam}} }); - if(response.ok){ - return response; - }; - throw await ResponseError.fromResponse(response); } {{/operation}}