From 3ef63171a9ce29a1027ee7ac571798ec5e374d73 Mon Sep 17 00:00:00 2001 From: Ben Reinhart Date: Tue, 20 Apr 2021 17:28:08 -0700 Subject: [PATCH] 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 | 32 +++++++++++++++++ superset-websocket/package.json | 2 ++ superset-websocket/src/index.ts | 35 +++++++------------ superset-websocket/src/logger.ts | 51 ++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 22 deletions(-) create mode 100644 superset-websocket/src/logger.ts diff --git a/superset-websocket/package-lock.json b/superset-websocket/package-lock.json index 87af55074..10fea2eac 100644 --- a/superset-websocket/package-lock.json +++ b/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", diff --git a/superset-websocket/package.json b/superset-websocket/package.json index a26dfc253..d88fbd5a4 100644 --- a/superset-websocket/package.json +++ b/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", diff --git a/superset-websocket/src/index.ts b/superset-websocket/src/index.ts index 14ae5f829..21de55f5e 100644 --- a/superset-websocket/src/index.ts +++ b/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'); -const jwt = require('jsonwebtoken'); -const cookie = require('cookie'); -const Redis = require('ioredis'); +import { createLogger } from './logger'; export type StreamResult = [ recordId: string, @@ -114,20 +114,11 @@ try { Object.assign(opts, config); // init logger -const logTransports = [ - new winston.transports.Console({ handleExceptions: true }), -]; -if (opts.logToFile && opts.logFilename) { - logTransports.push( - new winston.transports.File({ - filename: opts.logFilename, - handleExceptions: true, - }), - ); -} -const logger = winston.createLogger({ - level: opts.logLevel, - transports: logTransports, +const logger = createLogger({ + silent: environment === 'test', + logLevel: opts.logLevel, + logToFile: opts.logToFile, + logFilename: opts.logFilename, }); // 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; }; /** diff --git a/superset-websocket/src/logger.ts b/superset-websocket/src/logger.ts new file mode 100644 index 000000000..c6efc2a50 --- /dev/null +++ b/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 = [ + 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, + }); +}