fix: handle upgrade properly in signal router

Signed-off-by: Heikki Hellgren <heikki.hellgren@op.fi>

plugins/devtools-backend/api-report.md

@@ -25,6 +25,8 @@ export class DevToolsBackendApi {
   listExternalDependencyDetails(): Promise<ExternalDependency[]>;
   // (undocumented)
   listInfo(): Promise<DevToolsInfo>;
+  // (undocumented)
+  listResourceUtilization(): Promise<string>;
 }
 
 // @public

plugins/devtools-backend/src/api/DevToolsBackendApi.ts

@@ -203,17 +203,21 @@ export class DevToolsBackendApi {
     return configInfo;
   }
 
-  public async listInfo(): Promise<DevToolsInfo> {
-    const operatingSystem = `${os.hostname()}: ${os.type} ${os.release} - ${
-      os.platform
-    }/${os.arch}`;
+  public async listResourceUtilization(): Promise<string> {
     const usedMem = Math.floor((os.totalmem() - os.freemem()) / (1024 * 1024));
-    const resources = `Memory: ${usedMem}/${Math.floor(
+    return `Memory: ${usedMem}/${Math.floor(
       os.totalmem() / (1024 * 1024),
     )}MB - Load: ${os
       .loadavg()
       .map(v => v.toFixed(2))
       .join('/')}`;
+  }
+
+  public async listInfo(): Promise<DevToolsInfo> {
+    const operatingSystem = `${os.hostname()}: ${os.type} ${os.release} - ${
+      os.platform
+    }/${os.arch}`;
+
     const nodeJsVersion = process.version;
 
     /* eslint-disable-next-line no-restricted-syntax */
@@ -247,7 +251,7 @@ export class DevToolsBackendApi {
 
     const info: DevToolsInfo = {
       operatingSystem: operatingSystem ?? 'N/A',
-      resourceUtilization: resources ?? 'N/A',
+      resourceUtilization: (await this.listResourceUtilization()) ?? 'N/A',
       nodeJsVersion: nodeJsVersion ?? 'N/A',
       backstageVersion:
         backstageJson && backstageJson.version ? backstageJson.version : 'N/A',

plugins/devtools-backend/src/service/router.ts

@@ -56,9 +56,11 @@ export async function createRouter(
   if (signalService) {
     // Publish info periodically using the signal service
     setInterval(async () => {
-      if (signalService.hasSubscribers('devtools:info')) {
-        const info = await devToolsBackendApi.listInfo();
-        await signalService.publish('*', 'devtools:info', info);
+      if (signalService.hasSubscribers('devtools:resources')) {
+        const info = await devToolsBackendApi.listResourceUtilization();
+        await signalService.publish('*', 'devtools:resources', {
+          resources: info,
+        });
       }
     }, 5000);
   }

plugins/devtools/src/components/Content/InfoContent/InfoContent.tsx

@@ -73,15 +73,15 @@ const copyToClipboard = ({ about }: { about: DevToolsInfo | undefined }) => {
 /** @public */
 export const InfoContent = () => {
   const classes = useStyles();
-  const [info, setInfo] = useState<DevToolsInfo | undefined>(undefined);
+  const [resources, setResources] = useState<string | undefined>(undefined);
   const { about, loading, error } = useInfo();
-  useSignalApi('devtools:info', message => {
-    setInfo(message as DevToolsInfo);
+  useSignalApi('devtools:resources', message => {
+    setResources(message.resources as string);
   });
 
   useEffect(() => {
     if (!loading && !error && about) {
-      setInfo(about);
+      setResources(about.resourceUtilization);
     }
   }, [about, loading, error]);
 
@@ -102,7 +102,7 @@ export const InfoContent = () => {
             </ListItemAvatar>
             <ListItemText
               primary="Operating System"
-              secondary={info?.operatingSystem}
+              secondary={about?.operatingSystem}
             />
           </ListItem>
           <ListItem>
@@ -113,7 +113,7 @@ export const InfoContent = () => {
             </ListItemAvatar>
             <ListItemText
               primary="Resource utilization"
-              secondary={info?.resourceUtilization}
+              secondary={resources}
             />
           </ListItem>
           <ListItem>
@@ -124,7 +124,7 @@ export const InfoContent = () => {
             </ListItemAvatar>
             <ListItemText
               primary="NodeJS Version"
-              secondary={info?.nodeJsVersion}
+              secondary={about?.nodeJsVersion}
             />
           </ListItem>
           <ListItem>
@@ -135,14 +135,14 @@ export const InfoContent = () => {
             </ListItemAvatar>
             <ListItemText
               primary="Backstage Version"
-              secondary={info?.backstageVersion}
+              secondary={about?.backstageVersion}
             />
           </ListItem>
           <Divider orientation="vertical" variant="middle" flexItem />
           <ListItem
             button
             onClick={() => {
-              copyToClipboard({ about: info });
+              copyToClipboard({ about });
             }}
             className={classes.copyButton}
           >
@@ -155,7 +155,7 @@ export const InfoContent = () => {
           </ListItem>
         </List>
       </Paper>
-      <InfoDependenciesTable infoDependencies={info?.dependencies} />
+      <InfoDependenciesTable infoDependencies={about?.dependencies} />
     </Box>
   );
 };

plugins/signals-backend/src/service/router.ts

@@ -14,10 +14,12 @@
  * limitations under the License.
  */
 import { errorHandler } from '@backstage/backend-common';
-import express from 'express';
+import express, { NextFunction, Request, Response } from 'express';
 import Router from 'express-promise-router';
 import { Logger } from 'winston';
 import { SignalService } from '@backstage/plugin-signals-node';
+import * as https from 'https';
+import http from 'http';
 
 /** @public */
 export interface RouterOptions {
@@ -30,17 +32,14 @@ export async function createRouter(
   options: RouterOptions,
 ): Promise<express.Router> {
   const { logger, service } = options;
+  let subscribed = false;
 
-  const router = Router();
-  router.use(express.json());
-
-  router.get('/health', (_, response) => {
-    logger.info('PONG!');
-    response.json({ status: 'ok' });
-  });
-
-  router.get('/', async (req, _, next) => {
+  const upgradeMiddleware = (req: Request, _: Response, next: NextFunction) => {
+    const server: https.Server | http.Server = (
+      (req.socket ?? req.connection) as any
+    )?.server;
     if (
+      !server ||
       !req.headers ||
       req.headers.upgrade === undefined ||
       req.headers.upgrade.toLowerCase() !== 'websocket'
@@ -49,7 +48,21 @@ export async function createRouter(
       return;
     }
 
-    await service.handleUpgrade(req);
+    if (!subscribed) {
+      server.on('upgrade', async (request, socket, head) => {
+        await service.handleUpgrade(request, socket, head);
+      });
+      subscribed = true;
+    }
+  };
+
+  const router = Router();
+  router.use(express.json());
+  router.use(upgradeMiddleware);
+
+  router.get('/health', (_, response) => {
+    logger.info('PONG!');
+    response.json({ status: 'ok' });
   });
 
   router.use(errorHandler());

plugins/signals-node/api-report.md

@@ -3,13 +3,16 @@
 > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
 
 ```ts
+/// <reference types="node" />
+
+import { Duplex } from 'stream';
 import { EventBroker } from '@backstage/plugin-events-node';
 import { EventParams } from '@backstage/plugin-events-node';
 import { EventSubscriber } from '@backstage/plugin-events-node';
 import { IdentityApi } from '@backstage/plugin-auth-node';
+import { IncomingMessage } from 'http';
 import { JsonObject } from '@backstage/types';
 import { Logger } from 'winston';
-import { Request as Request_2 } from 'express';
 
 // @public (undocumented)
 export type ServiceOptions = {
@@ -29,7 +32,11 @@ export type SignalEventBrokerPayload = {
 export class SignalService implements EventSubscriber {
   // (undocumented)
   static create(options: ServiceOptions): SignalService;
-  handleUpgrade: (req: Request_2) => Promise<void>;
+  handleUpgrade: (
+    req: IncomingMessage,
+    socket: Duplex,
+    head: Buffer,
+  ) => Promise<void>;
   hasSubscribers(topic: string): boolean;
   // (undocumented)
   onEvent(params: EventParams<SignalEventBrokerPayload>): Promise<void>;

plugins/signals-node/src/SignalService.ts

@@ -27,13 +27,13 @@ import {
 import { RawData, WebSocket, WebSocketServer } from 'ws';
 import { IncomingMessage } from 'http';
 import { v4 as uuid } from 'uuid';
-import { Request } from 'express';
 import { JsonObject } from '@backstage/types';
 import {
   BackstageIdentityResponse,
   IdentityApi,
   IdentityApiGetIdentityRequest,
 } from '@backstage/plugin-auth-node';
+import { Duplex } from 'stream';
 
 /** @public */
 export class SignalService implements EventSubscriber {
@@ -61,14 +61,7 @@ export class SignalService implements EventSubscriber {
     this.serverId = uuid();
     this.server = new WebSocketServer({
       noServer: true,
-    });
-
-    this.server.on('close', () => {
-      this.logger.info('Closing signals server');
-      this.connections.forEach(conn => {
-        conn.ws.close();
-      });
-      this.connections = new Map();
+      clientTracking: false,
     });
 
     this.eventBroker?.subscribe(this);
@@ -79,7 +72,11 @@ export class SignalService implements EventSubscriber {
    * list for publish/subscribe functionality
    * @param req - Request
    */
-  handleUpgrade = async (req: Request) => {
+  handleUpgrade = async (
+    req: IncomingMessage,
+    socket: Duplex,
+    head: Buffer,
+  ) => {
     let identity: BackstageIdentityResponse | undefined = undefined;
 
     // Authentication token is passed in Sec-WebSocket-Protocol header as there
@@ -95,8 +92,8 @@ export class SignalService implements EventSubscriber {
 
     this.server.handleUpgrade(
       req,
-      req.socket,
-      Buffer.from(''),
+      socket,
+      head,
       (ws: WebSocket, __: IncomingMessage) => {
         this.addConnection(ws, identity);
       },
@@ -131,17 +128,11 @@ export class SignalService implements EventSubscriber {
       this.connections.delete(id);
     });
 
-    ws.on('ping', () => {
-      this.logger.debug(`Ping from connection ${id}`);
-      ws.pong();
-    });
-
     ws.on('message', (data: RawData, isBinary: boolean) => {
       this.logger.debug(`Received message from connection ${id}: ${data}`);
       if (isBinary) {
         return;
       }
-
       try {
         const json = JSON.parse(data.toString()) as JsonObject;
         this.handleMessage(conn, json);