From dcba4404597d60d63a267138e36eef2a40750bf7 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 4 Jul 2022 15:52:18 +0200 Subject: [PATCH] refactor: apply review suggestions Signed-off-by: Camila Belo --- .changeset/two-owls-cry.md | 1 - .../src/engines/ElasticSearchSearchEngine.test.ts | 2 +- .../src/PgSearchEngine/PgSearchEngine.test.ts | 15 --------------- .../src/PgSearchEngine/PgSearchEngine.ts | 7 ------- .../src/engines/LunrSearchEngine.test.ts | 2 +- .../src/engines/LunrSearchEngine.ts | 2 +- plugins/search-backend/src/service/router.ts | 14 ++++++-------- 7 files changed, 9 insertions(+), 34 deletions(-) diff --git a/.changeset/two-owls-cry.md b/.changeset/two-owls-cry.md index b469561ad7..ca371781a1 100644 --- a/.changeset/two-owls-cry.md +++ b/.changeset/two-owls-cry.md @@ -1,6 +1,5 @@ --- '@backstage/plugin-search-backend-module-elasticsearch': patch -'@backstage/plugin-search-backend-module-pg': patch --- Throws `MissingIndexError` when no index of type exist. diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts index d794ea668d..b59c67c69c 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts @@ -813,7 +813,7 @@ describe('ElasticSearchSearchEngine', () => { filters: {}, }), ).rejects.toThrow( - 'Missing index for unknown__search. This means there are no documents to search through', + 'Missing index for unknown__search. This means there are no documents to search through.', ); }); }); diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts index e106cd6f06..3ed002567e 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts @@ -271,21 +271,6 @@ describe('PgSearchEngine', () => { limit: 26, }); }); - - it('should throws missing index error', async () => { - database.transaction.mockImplementation(fn => fn(tx)); - database.query.mockResolvedValue([]); - await expect( - async () => - await searchEngine.query({ - term: 'testTerm', - types: ['unknown'], - filters: {}, - }), - ).rejects.toThrow( - 'Missing index for unknown. This means there are no documents to search through', - ); - }); }); }); diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts index 065f25878c..bf98c58de9 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts @@ -15,7 +15,6 @@ */ import { PluginDatabaseManager } from '@backstage/backend-common'; import { SearchEngine } from '@backstage/plugin-search-backend-node'; -import { MissingIndexError } from '@backstage/plugin-search-backend-node'; import { SearchQuery, IndexableResultSet, @@ -93,12 +92,6 @@ export class PgSearchEngine implements SearchEngine { this.databaseStore.query(tx, pgQuery), ); - if (pgQuery?.types && !rows.length) { - throw new MissingIndexError( - `Missing index for ${pgQuery.types}. This means there are no documents to search through.`, - ); - } - // We requested one result more than the page size to know whether there is // another page. const { page } = decodePageCursor(query.pageCursor); diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts index 0bc2e223a6..1f69460448 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts @@ -959,7 +959,7 @@ describe('LunrSearchEngine', () => { filters: {}, }), ).rejects.toThrow( - 'Missing index for unknown. This means there are no documents to search through', + "Missing index for unknown. This could be because the index hasn't been created yet or there was a problem during index creation.", ); }); }); diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts index a06a565d3b..ec8550d721 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts @@ -170,7 +170,7 @@ export class LunrSearchEngine implements SearchEngine { if (documentTypes?.length && !indexKeys.length) { throw new MissingIndexError( - `Missing index for ${documentTypes?.toString()}. This means there are no documents to search through.`, + `Missing index for ${documentTypes?.toString()}. This could be because the index hasn't been created yet or there was a problem during index creation.`, ); } diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 0fbacb018e..e4ac233406 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -19,7 +19,7 @@ import Router from 'express-promise-router'; import { Logger } from 'winston'; import { z } from 'zod'; import { errorHandler } from '@backstage/backend-common'; -import { InputError } from '@backstage/errors'; +import { ErrorResponseBody, InputError } from '@backstage/errors'; import { Config } from '@backstage/config'; import { JsonObject, JsonValue } from '@backstage/types'; import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; @@ -33,10 +33,7 @@ import { IndexableResultSet, SearchResultSet, } from '@backstage/plugin-search-common'; -import { - SearchEngine, - MissingIndexError, -} from '@backstage/plugin-search-backend-node'; +import { SearchEngine } from '@backstage/plugin-search-backend-node'; import { AuthorizedSearchEngine } from './AuthorizedSearchEngine'; const jsonObjectSchema: z.ZodSchema = z.lazy(() => { @@ -134,7 +131,7 @@ export async function createRouter( '/query', async ( req: express.Request, - res: express.Response, + res: express.Response, ) => { const parseResult = requestSchema.safeParse(req.query); @@ -161,8 +158,9 @@ export async function createRouter( res.send(filterResultSet(toSearchResults(resultSet))); } catch (error) { - if (error instanceof MissingIndexError) { - res.status(400).json(error.message); + if (error.name === 'MissingIndexError') { + // re-throw and let the default error handler middleware captures it and serializes it with the right response code on the standard form + throw error; } throw new Error(