chore: WebSocket server improvements (#14257)

* Update some types and resolve conflicts

* Logger improvements

* Ensure logger can log exception objects
* Remove logging output in tests (for now)

* Add License

superset-websocket/package-lock.json

@@ -16,8 +16,10 @@
         "ws": "^7.4.2"
       },
       "devDependencies": {
+        "@types/cookie": "^0.4.0",
         "@types/ioredis": "^4.22.0",
         "@types/jest": "^26.0.20",
+        "@types/jsonwebtoken": "^8.5.1",
         "@types/node": "^14.14.22",
         "@types/uuid": "^8.3.0",
         "@types/ws": "^7.4.0",
@@ -898,6 +900,12 @@
         "@babel/types": "^7.3.0"
       }
     },
+    "node_modules/@types/cookie": {
+      "version": "0.4.0",
+      "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.4.0.tgz",
+      "integrity": "sha512-y7mImlc/rNkvCRmg8gC3/lj87S7pTUIJ6QGjwHR9WQJcFs+ZMTOaoPrkdFA/YdbuqVEmEbb5RdhVxMkAcgOnpg==",
+      "dev": true
+    },
     "node_modules/@types/graceful-fs": {
       "version": "4.1.5",
       "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.5.tgz",
@@ -956,6 +964,15 @@
       "integrity": "sha512-cxWFQVseBm6O9Gbw1IWb8r6OS4OhSt3hPZLkFApLjM8TEXROBuQGLAH2i2gZpcXdLBIrpXuTDhH7Vbm1iXmNGA==",
       "dev": true
     },
+    "node_modules/@types/jsonwebtoken": {
+      "version": "8.5.1",
+      "resolved": "https://registry.npmjs.org/@types/jsonwebtoken/-/jsonwebtoken-8.5.1.tgz",
+      "integrity": "sha512-rNAPdomlIUX0i0cg2+I+Q1wOUr531zHBQ+cV/28PJ39bSPKjahatZZ2LMuhiguETkCgLVzfruw/ZvNMNkKoSzw==",
+      "dev": true,
+      "dependencies": {
+        "@types/node": "*"
+      }
+    },
     "node_modules/@types/node": {
       "version": "14.14.22",
       "resolved": "https://registry.npmjs.org/@types/node/-/node-14.14.22.tgz",
@@ -8180,6 +8197,12 @@
         "@babel/types": "^7.3.0"
       }
     },
+    "@types/cookie": {
+      "version": "0.4.0",
+      "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.4.0.tgz",
+      "integrity": "sha512-y7mImlc/rNkvCRmg8gC3/lj87S7pTUIJ6QGjwHR9WQJcFs+ZMTOaoPrkdFA/YdbuqVEmEbb5RdhVxMkAcgOnpg==",
+      "dev": true
+    },
     "@types/graceful-fs": {
       "version": "4.1.5",
       "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.5.tgz",
@@ -8238,6 +8261,15 @@
       "integrity": "sha512-cxWFQVseBm6O9Gbw1IWb8r6OS4OhSt3hPZLkFApLjM8TEXROBuQGLAH2i2gZpcXdLBIrpXuTDhH7Vbm1iXmNGA==",
       "dev": true
     },
+    "@types/jsonwebtoken": {
+      "version": "8.5.1",
+      "resolved": "https://registry.npmjs.org/@types/jsonwebtoken/-/jsonwebtoken-8.5.1.tgz",
+      "integrity": "sha512-rNAPdomlIUX0i0cg2+I+Q1wOUr531zHBQ+cV/28PJ39bSPKjahatZZ2LMuhiguETkCgLVzfruw/ZvNMNkKoSzw==",
+      "dev": true,
+      "requires": {
+        "@types/node": "*"
+      }
+    },
     "@types/node": {
       "version": "14.14.22",
       "resolved": "https://registry.npmjs.org/@types/node/-/node-14.14.22.tgz",

superset-websocket/package.json

@@ -23,8 +23,10 @@
     "ws": "^7.4.2"
   },
   "devDependencies": {
+    "@types/cookie": "^0.4.0",
     "@types/ioredis": "^4.22.0",
     "@types/jest": "^26.0.20",
+    "@types/jsonwebtoken": "^8.5.1",
     "@types/node": "^14.14.22",
     "@types/uuid": "^8.3.0",
     "@types/ws": "^7.4.0",

superset-websocket/src/index.ts

@@ -20,11 +20,11 @@ import * as http from 'http';
 import * as net from 'net';
 import WebSocket from 'ws';
 import { v4 as uuidv4 } from 'uuid';
+import jwt from 'jsonwebtoken';
+import cookie from 'cookie';
+import Redis from 'ioredis';
 
-const winston = require('winston');
+import { createLogger } from './logger';
-const jwt = require('jsonwebtoken');
-const cookie = require('cookie');
-const Redis = require('ioredis');
 
 export type StreamResult = [
   recordId: string,
@@ -114,20 +114,11 @@ try {
 Object.assign(opts, config);
 
 // init logger
-const logTransports = [
+const logger = createLogger({
-  new winston.transports.Console({ handleExceptions: true }),
+  silent: environment === 'test',
-];
+  logLevel: opts.logLevel,
-if (opts.logToFile && opts.logFilename) {
+  logToFile: opts.logToFile,
-  logTransports.push(
+  logFilename: opts.logFilename,
-    new winston.transports.File({
-      filename: opts.logFilename,
-      handleExceptions: true,
-    }),
-  );
-}
-const logger = winston.createLogger({
-  level: opts.logLevel,
-  transports: logTransports,
 });
 
 // enforce JWT secret length
@@ -219,7 +210,7 @@ export const fetchRangeFromStream = async ({
   try {
     const reply = await redis.xrange(streamName, startId, endId);
     if (!reply || !reply.length) return;
-    listener(reply);
+    listener(reply as StreamResult[]);
   } catch (e) {
     logger.error(e);
   }
@@ -254,7 +245,7 @@ export const subscribeToGlobalStream = async (
       if (!results.length) {
         continue;
       }
-      listener(results);
+      listener(results as StreamResult[]);
       setLastFirehoseId(results[length - 1][0]);
     } catch (e) {
       logger.error(e);
@@ -284,11 +275,11 @@ export const processStreamResults = (results: StreamResult[]): void => {
  * Returns the JWT payload or throws an error on invalid token.
  */
 const getJwtPayload = (request: http.IncomingMessage): JwtPayload => {
-  const cookies = cookie.parse(request.headers.cookie);
+  const cookies = cookie.parse(request.headers.cookie || '');
   const token = cookies[opts.jwtCookieName];
 
   if (!token) throw new Error('JWT not present');
-  return jwt.verify(token, opts.jwtSecret);
+  return jwt.verify(token, opts.jwtSecret) as JwtPayload;
 };
 
 /**

superset-websocket/src/logger.ts

@@ -0,0 +1,51 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import winston from 'winston';
+
+interface LoggingOptionsType {
+  silent: boolean;
+  logLevel: string;
+  logToFile: boolean;
+  logFilename: string;
+}
+
+export function createLogger(opts: LoggingOptionsType) {
+  const logTransports: Array<winston.transport> = [
+    new winston.transports.Console({ handleExceptions: true }),
+  ];
+
+  if (opts.logToFile && opts.logFilename) {
+    logTransports.push(
+      new winston.transports.File({
+        filename: opts.logFilename,
+        handleExceptions: true,
+      }),
+    );
+  }
+
+  return winston.createLogger({
+    level: opts.logLevel,
+    transports: logTransports,
+    format: winston.format.combine(
+      winston.format.errors({ stack: true }),
+      winston.format.json(),
+    ),
+    silent: opts.silent,
+  });
+}