Merge pull request #24861 from alexef/new-events-implementation

fix: move from event-source-polyfill to fetch-event-source
backstage · Jun 3, 2024 · 42f1865 · 42f1865
2 parents eee7474 + 9d54ebf
commit 42f1865
Show file tree

Hide file tree

Showing 14 changed files with 152 additions and 264 deletions.
diff --git a/.changeset/empty-avocados-tease.md b/.changeset/empty-avocados-tease.md
@@ -0,0 +1,6 @@
+---
+'@backstage/plugin-scaffolder': patch
+'@backstage/plugin-techdocs': patch
+---
+
+Internal updates to allow reusing Backstage's `fetchApi` implementation for event source requests. This allows you to for example, override the `Authorization` header.
diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json
@@ -70,6 +70,7 @@
     "@material-ui/core": "^4.12.2",
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "4.0.0-alpha.61",
+    "@microsoft/fetch-event-source": "^2.0.1",
     "@react-hookz/web": "^24.0.0",
     "@rjsf/core": "5.18.2",
     "@rjsf/material-ui": "5.18.2",
@@ -78,7 +79,6 @@
     "@types/react": "^16.13.1 || ^17.0.0 || ^18.0.0",
     "@uiw/react-codemirror": "^4.9.3",
     "classnames": "^2.2.6",
-    "event-source-polyfill": "^1.0.31",
     "git-url-parse": "^14.0.0",
     "humanize-duration": "^3.25.1",
     "json-schema": "^0.4.0",

diff --git a/plugins/scaffolder/src/api.test.ts b/plugins/scaffolder/src/api.test.ts
@@ -20,14 +20,13 @@ import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils';
 import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import { ScaffolderClient } from './api';
-import { EventSourcePolyfill } from 'event-source-polyfill';
+import { fetchEventSource } from '@microsoft/fetch-event-source';
 
-const MockedEventSource = EventSourcePolyfill as jest.MockedClass<
-  typeof EventSourcePolyfill
+jest.mock('@microsoft/fetch-event-source');
+const mockFetchEventSource = fetchEventSource as jest.MockedFunction<
+  typeof fetchEventSource
 >;
 
-jest.mock('event-source-polyfill');
-
 const server = setupServer();
 
 describe('api', () => {
@@ -87,26 +86,23 @@ describe('api', () => {
   describe('streamEvents', () => {
     describe('eventsource', () => {
       it('should work', async () => {
-        MockedEventSource.prototype.addEventListener.mockImplementation(
-          (type, fn) => {
-            if (typeof fn !== 'function') {
-              return;
-            }
-
-            if (type === 'log') {
-              fn({
-                data: '{"id":1,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}}',
-              } as any);
-            } else if (type === 'completion') {
-              fn({
-                data: '{"id":2,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}}',
-              } as any);
-            }
-          },
-        );
-
-        const token = 'fake-token';
-        identityApi.getCredentials.mockResolvedValue({ token: token });
+        mockFetchEventSource.mockImplementation(async (_url, options) => {
+          const { onopen, onmessage } = options;
+          await Promise.resolve();
+          await onopen?.({ ok: true } as Response);
+          await Promise.resolve();
+          onmessage?.({
+            id: '',
+            event: 'log',
+            data: '{"id":1,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}}',
+          });
+          await Promise.resolve();
+          onmessage?.({
+            id: '',
+            event: 'completion',
+            data: '{"id":2,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}}',
+          });
+        });
 
         const next = jest.fn();
 
@@ -116,14 +112,15 @@ describe('api', () => {
             .subscribe({ next, complete });
         });
 
-        expect(MockedEventSource).toHaveBeenCalledWith(
+        expect(mockFetchEventSource).toHaveBeenCalledWith(
           'http://backstage/api/v2/tasks/a-random-task-id/eventstream',
           {
-            withCredentials: true,
-            headers: { Authorization: `Bearer ${token}` },
+            fetch: fetchApi.fetch,
+            onmessage: expect.any(Function),
+            onerror: expect.any(Function),
+            signal: expect.any(AbortSignal),
           },
         );
-        expect(MockedEventSource.prototype.close).toHaveBeenCalled();
 
         expect(next).toHaveBeenCalledTimes(2);
         expect(next).toHaveBeenCalledWith({

diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts
@@ -41,7 +41,10 @@ import {
 } from '@backstage/plugin-scaffolder-react';
 
 import queryString from 'qs';
-import { EventSourcePolyfill } from 'event-source-polyfill';
+import {
+  EventSourceMessage,
+  fetchEventSource,
+} from '@microsoft/fetch-event-source';
 
 /**
  * An API to interact with the scaffolder backend.
@@ -226,11 +229,8 @@ export class ScaffolderClient implements ScaffolderApi {
         params.set('after', String(Number(after)));
       }
 
-      Promise.all([
-        this.discoveryApi.getBaseUrl('scaffolder'),
-        this.identityApi?.getCredentials(),
-      ]).then(
-        ([baseUrl, credentials]) => {
+      this.discoveryApi.getBaseUrl('scaffolder').then(
+        baseUrl => {
           const url = `${baseUrl}/v2/tasks/${encodeURIComponent(
             taskId,
           )}/eventstream`;
@@ -245,22 +245,25 @@ export class ScaffolderClient implements ScaffolderApi {
             }
           };
 
-          const eventSource = new EventSourcePolyfill(url, {
-            withCredentials: true,
-            headers: credentials?.token
-              ? { Authorization: `Bearer ${credentials.token}` }
-              : {},
-          });
-          eventSource.addEventListener('log', processEvent);
-          eventSource.addEventListener('recovered', processEvent);
-          eventSource.addEventListener('cancelled', processEvent);
-          eventSource.addEventListener('completion', (event: any) => {
-            processEvent(event);
-            eventSource.close();
-            subscriber.complete();
-          });
-          eventSource.addEventListener('error', event => {
-            subscriber.error(event);
+          const ctrl = new AbortController();
+          fetchEventSource(url, {
+            fetch: this.fetchApi.fetch,
+            signal: ctrl.signal,
+            onmessage(e: EventSourceMessage) {
+              if (e.event === 'log') {
+                processEvent(e);
+                return;
+              } else if (e.event === 'completion') {
+                processEvent(e);
+                subscriber.complete();
+                ctrl.abort();
+                return;
+              }
+              processEvent(e);
+            },
+            onerror(err) {
+              subscriber.error(err);
+            },
           });
         },
         error => {

diff --git a/plugins/scaffolder/src/setupTests.ts b/plugins/scaffolder/src/setupTests.ts
@@ -16,9 +16,6 @@
 
 import '@testing-library/jest-dom';
 
-const { EventSourcePolyfill } = jest.requireMock('event-source-polyfill');
-global.EventSource = EventSourcePolyfill;
-
 // Patch jsdom to add feature used by CodeMirror
 document.createRange = () => {
   const range = new Range();

diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md
@@ -465,8 +465,8 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 {
   constructor(options: {
     configApi: Config;
     discoveryApi: DiscoveryApi;
-    identityApi: IdentityApi;
     fetchApi: FetchApi;
+    identityApi?: IdentityApi;
   });
   // (undocumented)
   configApi: Config;
@@ -485,8 +485,6 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 {
   getEntityDocs(entityId: CompoundEntityRef, path: string): Promise<string>;
   // (undocumented)
   getStorageUrl(): Promise<string>;
-  // (undocumented)
-  identityApi: IdentityApi;
   syncEntityDocs(
     entityId: CompoundEntityRef,
     logHandler?: (line: string) => void,

diff --git a/plugins/techdocs/dev/index.tsx b/plugins/techdocs/dev/index.tsx
@@ -25,11 +25,7 @@ import {
   techdocsStorageApiRef,
 } from '../src';
 
-import {
-  configApiRef,
-  discoveryApiRef,
-  identityApiRef,
-} from '@backstage/core-plugin-api';
+import { configApiRef, discoveryApiRef } from '@backstage/core-plugin-api';
 import { TechDocsReaderPageProvider } from '@backstage/plugin-techdocs-react';
 import { Header, Page, TabbedLayout } from '@backstage/core-components';
 
@@ -135,7 +131,6 @@ createDevApp()
     deps: {
       configApi: configApiRef,
       discoveryApi: discoveryApiRef,
-      identityApi: identityApiRef,
     },
     factory: () => apiBridge,
   })

diff --git a/plugins/techdocs/package.json b/plugins/techdocs/package.json
@@ -70,9 +70,9 @@
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "4.0.0-alpha.61",
     "@material-ui/styles": "^4.10.0",
+    "@microsoft/fetch-event-source": "^2.0.1",
     "@types/react": "^16.13.1 || ^17.0.0 || ^18.0.0",
     "dompurify": "^3.0.0",
-    "event-source-polyfill": "1.0.25",
     "git-url-parse": "^14.0.0",
     "jss": "~10.10.0",
     "lodash": "^4.17.21",
@@ -90,7 +90,6 @@
     "@testing-library/react": "^15.0.0",
     "@testing-library/user-event": "^14.0.0",
     "@types/dompurify": "^3.0.0",
-    "@types/event-source-polyfill": "^1.0.0",
     "canvas": "^2.10.2"
   },
   "peerDependencies": {

diff --git a/plugins/techdocs/src/alpha.tsx b/plugins/techdocs/src/alpha.tsx
@@ -29,7 +29,6 @@ import {
   createApiFactory,
   discoveryApiRef,
   fetchApiRef,
-  identityApiRef,
 } from '@backstage/core-plugin-api';
 import {
   compatWrapper,
@@ -55,14 +54,12 @@ const techDocsStorageApi = createApiExtension({
     deps: {
       configApi: configApiRef,
       discoveryApi: discoveryApiRef,
-      identityApi: identityApiRef,
       fetchApi: fetchApiRef,
     },
-    factory: ({ configApi, discoveryApi, identityApi, fetchApi }) =>
+    factory: ({ configApi, discoveryApi, fetchApi }) =>
       new TechDocsStorageClient({
         configApi,
         discoveryApi,
-        identityApi,
         fetchApi,
       }),
   }),