From b78e3fdb718f865f71c8a06d2874ec52a433f7cc Mon Sep 17 00:00:00 2001 From: Alex Lorenzi Date: Wed, 12 Feb 2025 15:45:10 -0500 Subject: [PATCH 1/5] When the catalog entity couldn't be found the /sync endpoint would return with a 404 and a JSON response which was not expected by the event-stream fetch on the frontend. This causes the client to continuously make requests to the /sync endpoint. Signed-off-by: Alex Lorenzi --- .changeset/good-flowers-promise.md | 5 +++++ plugins/techdocs-backend/src/service/router.ts | 12 ++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 .changeset/good-flowers-promise.md diff --git a/.changeset/good-flowers-promise.md b/.changeset/good-flowers-promise.md new file mode 100644 index 0000000000..659c1ab48e --- /dev/null +++ b/.changeset/good-flowers-promise.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-backend': patch +--- + +Modified response when catalog entity isn't found to resolve issue where `/sync` endpoint was continuously called diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index b99579e0eb..4d5ba0ebf3 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -242,14 +242,14 @@ export async function createRouter( targetPluginId: 'catalog', }); - const entity = await entityLoader.load({ kind, namespace, name }, token); - - if (!entity?.metadata?.uid) { - throw new NotFoundError('Entity metadata UID missing'); - } - const responseHandler: DocsSynchronizerSyncOpts = createEventStream(res); + const entity = await entityLoader.load({ kind, namespace, name }, token); + if (!entity?.metadata?.uid) { + responseHandler.error(new NotFoundError('Entity metadata UID missing')); + return; + } + // By default, techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to // 'local'. If set to 'external', it will assume that an external process (e.g. CI/CD pipeline // of the repository) is responsible for building and publishing documentation to the storage provider. From 5cab95a0b32a80f423084628794889bdafad49eb Mon Sep 17 00:00:00 2001 From: Alex Lorenzi Date: Wed, 12 Feb 2025 16:13:18 -0500 Subject: [PATCH 2/5] Fixed tests Signed-off-by: Alex Lorenzi --- .../src/service/router.test.ts | 18 ++++++++++++++++-- plugins/techdocs-backend/src/service/router.ts | 6 +++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index c74167552d..21b6dd5554 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -164,7 +164,14 @@ describe('createRouter', () => { .set('accept', 'text/event-stream') .send(); - expect(response.status).toBe(404); + expect(response.status).toBe(200); + expect(response.get('content-type')).toBe('text/event-stream'); + expect(response.text).toEqual( + `event: error +data: "Entity not found" + +`, + ); }); it('should return not found if entity has no uid', async () => { @@ -179,7 +186,14 @@ describe('createRouter', () => { .set('accept', 'text/event-stream') .send(); - expect(response.status).toBe(404); + expect(response.status).toBe(200); + expect(response.get('content-type')).toBe('text/event-stream'); + expect(response.text).toEqual( + `event: error +data: "Entity metadata UID missing" + +`, + ); }); it('should not check for an update when shouldBuild returns false', async () => { diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 4d5ba0ebf3..301bc43755 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -246,7 +246,11 @@ export async function createRouter( const entity = await entityLoader.load({ kind, namespace, name }, token); if (!entity?.metadata?.uid) { - responseHandler.error(new NotFoundError('Entity metadata UID missing')); + responseHandler.error( + new NotFoundError( + entity ? 'Entity metadata UID missing' : 'Entity not found', + ), + ); return; } From 060453e7011b8bc0ec93af3ddc255f58ca5b4dd7 Mon Sep 17 00:00:00 2001 From: Alex Lorenzi <671432+alexlorenzi@users.noreply.github.com> Date: Wed, 12 Feb 2025 18:23:35 -0500 Subject: [PATCH 3/5] Update good-flowers-promise.md Signed-off-by: Alex Lorenzi <671432+alexlorenzi@users.noreply.github.com> --- .changeset/good-flowers-promise.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/good-flowers-promise.md b/.changeset/good-flowers-promise.md index 659c1ab48e..109cee9718 100644 --- a/.changeset/good-flowers-promise.md +++ b/.changeset/good-flowers-promise.md @@ -2,4 +2,4 @@ '@backstage/plugin-techdocs-backend': patch --- -Modified response when catalog entity isn't found to resolve issue where `/sync` endpoint was continuously called +Fixed issue `syncEntityDocs` that would cause the `/sync` endpoint to be continuously called if the request fails From 840a2687fe903d99308a1c7f956548b6fa8a2850 Mon Sep 17 00:00:00 2001 From: Alex Lorenzi Date: Wed, 12 Feb 2025 18:22:44 -0500 Subject: [PATCH 4/5] This is a better fix. Docs for Azure/fetch-event-source mention you should re-throw the error in the onerror function to stop operation Signed-off-by: Alex Lorenzi --- .../src/service/router.test.ts | 18 ++---------------- plugins/techdocs-backend/src/service/router.ts | 12 ++++-------- plugins/techdocs/src/client.ts | 1 + 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index 21b6dd5554..c74167552d 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -164,14 +164,7 @@ describe('createRouter', () => { .set('accept', 'text/event-stream') .send(); - expect(response.status).toBe(200); - expect(response.get('content-type')).toBe('text/event-stream'); - expect(response.text).toEqual( - `event: error -data: "Entity not found" - -`, - ); + expect(response.status).toBe(404); }); it('should return not found if entity has no uid', async () => { @@ -186,14 +179,7 @@ data: "Entity not found" .set('accept', 'text/event-stream') .send(); - expect(response.status).toBe(200); - expect(response.get('content-type')).toBe('text/event-stream'); - expect(response.text).toEqual( - `event: error -data: "Entity metadata UID missing" - -`, - ); + expect(response.status).toBe(404); }); it('should not check for an update when shouldBuild returns false', async () => { diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 301bc43755..b99579e0eb 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -242,18 +242,14 @@ export async function createRouter( targetPluginId: 'catalog', }); - const responseHandler: DocsSynchronizerSyncOpts = createEventStream(res); - const entity = await entityLoader.load({ kind, namespace, name }, token); + if (!entity?.metadata?.uid) { - responseHandler.error( - new NotFoundError( - entity ? 'Entity metadata UID missing' : 'Entity not found', - ), - ); - return; + throw new NotFoundError('Entity metadata UID missing'); } + const responseHandler: DocsSynchronizerSyncOpts = createEventStream(res); + // By default, techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to // 'local'. If set to 'external', it will assume that an external process (e.g. CI/CD pipeline // of the repository) is responsible for building and publishing documentation to the storage provider. diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index d00be2a01f..9ea753b1e6 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -236,6 +236,7 @@ export class TechDocsStorageClient implements TechDocsStorageApi { onerror(err) { ctrl.abort(); reject(err); + throw err; // rethrow to stop the operation }, }); }); From 76c91c11cb08ee9713e46ccea42b17fc19b061d2 Mon Sep 17 00:00:00 2001 From: Alex Lorenzi Date: Thu, 13 Feb 2025 12:46:57 -0500 Subject: [PATCH 5/5] fixed test Signed-off-by: Alex Lorenzi --- plugins/techdocs/src/client.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/techdocs/src/client.test.ts b/plugins/techdocs/src/client.test.ts index f66587a071..70c5f3568e 100644 --- a/plugins/techdocs/src/client.test.ts +++ b/plugins/techdocs/src/client.test.ts @@ -180,7 +180,11 @@ describe('TechDocsStorageClient', () => { mockFetchEventSource.mockImplementation(async (_url, options) => { const { onerror } = options; - onerror?.(new NotFoundError('Some not found warning')); + try { + onerror?.(new NotFoundError('Some not found warning')); + } catch (e) { + // do nothing + } }); // we await later after we emitted the error @@ -202,7 +206,11 @@ describe('TechDocsStorageClient', () => { mockFetchEventSource.mockImplementation(async (_url, options) => { const { onerror } = options; - onerror?.(new Error('Some other error')); + try { + onerror?.(new Error('Some other error')); + } catch (e) { + // do nothing + } }); await expect(promise).rejects.toThrow(Error);