diff --git a/packages/jsonrpc/src/bulk.test.ts b/packages/jsonrpc/src/bulk.test.ts index be8ff8a..4e9db09 100644 --- a/packages/jsonrpc/src/bulk.test.ts +++ b/packages/jsonrpc/src/bulk.test.ts @@ -1,10 +1,11 @@ -import * as util from './bulk' +import { json } from 'body-parser' import express from 'express' -import {WithContext} from './types' -import {jsonrpc} from './express' -import {noopLogger} from './test-utils' -import {createClient} from './supertest' -import {json} from 'body-parser' +import { RequestHandlerParams } from 'express-serve-static-core' +import * as util from './bulk' +import { jsonrpc } from './express' +import { createClient } from './supertest' +import { noopLogger } from './test-utils' +import { WithContext } from './types' describe('util', () => { @@ -85,7 +86,7 @@ describe('util', () => { describe('bulkJSONRPC', () => { const getContext = () => ({userId: 10}) - function createApp(router: express.Router) { + function createApp(router: RequestHandlerParams) { const app = express() app.use(json()) app.use('/rpc', router) diff --git a/packages/jsonrpc/src/express.test.ts b/packages/jsonrpc/src/express.test.ts index 9fe2381..afd0e58 100644 --- a/packages/jsonrpc/src/express.test.ts +++ b/packages/jsonrpc/src/express.test.ts @@ -27,6 +27,7 @@ describe('jsonrpc', () => { const ensureLoggedIn = ensure(c => !!c.userId) + @ensureLoggedIn class MyService implements WithContext { constructor(readonly time: number) {} add(context: Context, a: number, b: number) { @@ -73,7 +74,7 @@ describe('jsonrpc', () => { app.use(bodyParser.json()) app.use('/', jsonrpc(() => ({userId}), noopLogger) - .addService('/myService', new MyService(5), [ + .addService('/app/myService', new MyService(5), [ 'add', 'delay', 'syncError', @@ -87,7 +88,7 @@ describe('jsonrpc', () => { return app } - const client = createClient(createApp(), '/myService') + const client = createClient(createApp(), '/app/myService') async function getError(promise: Promise) { let error @@ -103,7 +104,7 @@ describe('jsonrpc', () => { describe('errors', () => { it('handles sync errors', async () => { const response = await request(createApp()) - .post('/myService') + .post('/app/myService') .send({ id: 1, jsonrpc: '2.0', @@ -129,14 +130,14 @@ describe('jsonrpc', () => { }) it('returns an error when message is not readable', async () => { const result = await request(createApp()) - .post('/myService') + .post('/app/myService') .send('a=1') .expect(400) expect(result.body.error.message).toEqual('Invalid request') }) it('returns an error when message is not valid', async () => { const result = await request(createApp()) - .post('/myService') + .post('/app/myService') .send({}) .expect(400) expect(result.body.error.message).toEqual('Invalid request') @@ -171,7 +172,7 @@ describe('jsonrpc', () => { }) it('handles synchronous notifications', async () => { await request(createApp()) - .post('/myService') + .post('/app/myService') .send({ jsonrpc: '2.0', method: 'add', @@ -181,7 +182,7 @@ describe('jsonrpc', () => { .expect('') await request(createApp()) - .post('/myService') + .post('/app/myService') .send({ jsonrpc: '2.0', id: null, @@ -193,10 +194,110 @@ describe('jsonrpc', () => { }) }) + describe('invalid requests', () => { + it('returns 404 when invalid request method used', async () => { + await request(createApp()) + .put('/app/myService') + .send({ + id: 123, + jsonrpc: '2.0', + method: 'toString', + params: [], + }) + .expect(404) + }) + + it('returns 404 when service url is invalid', async () => { + await request(createApp()) + .post('/app/nonExistingService') + .send({ + id: 123, + jsonrpc: '2.0', + method: 'toString', + params: [], + }) + .expect(404) + }) + }) + + describe('multiple services', () => { + interface S1 { + add(a: number, b: number): number + } + class Test1 implements WithContext { + add(c: Context, a: number, b: number) { + return a + b + } + } + class Test2 implements WithContext { + add(c: Context, a: number, b: number): number { + throw new Error('Not implemented') + } + } + const app = express() + app.use(bodyParser.json()) + app.get('/app/s3', (req, res) => { + throw new Error('test s3') + }) + app.use('/app', + jsonrpc(() => ({userId: 1}), noopLogger) + .addService('/s1', new Test1(), ['add']) + .addService('/s2', new Test2(), ['add']) + .router(), + ) + + it('invokes the first service', async () => { + await request(app) + .post('/app/s1') + .send({ + id: 123, + jsonrpc: '2.0', + method: 'add', + params: [1, 2], + }) + .expect(200) + .expect({ + jsonrpc: '2.0', + id: 123, + result: 3, + error: null, + }) + }) + + it('invokes the second service', async () => { + await request(app) + .post('/app/s2') + .send({ + id: 123, + jsonrpc: '2.0', + method: 'add', + params: [1, 2], + }) + .expect(500) + .expect({ + jsonrpc: '2.0', + id: 123, + result: null, + error: { + code: -32000, + message: 'Server error', + data: null, + }, + }) + }) + + it('invokes the second service', async () => { + await request(app) + .get('/app/s3') + .expect(500) + .expect(/Error: test s3/) + }) + }) + describe('security', () => { it('cannot call toString method', async () => { await request(createApp()) - .post('/myService') + .post('/app/myService') .send({ id: 123, jsonrpc: '2.0', @@ -218,7 +319,7 @@ describe('jsonrpc', () => { it('cannot call private _-prefixed methods', async () => { await request(createApp()) - .post('/myService') + .post('/app/myService') .send({ id: 123, jsonrpc: '2.0', @@ -231,7 +332,7 @@ describe('jsonrpc', () => { it('cannot call any other methods in objects prototype', async () => { await request(createApp()) - .post('/myService') + .post('/app/myService') .send({ id: 123, jsonrpc: '2.0', @@ -254,7 +355,7 @@ describe('jsonrpc', () => { it('cannot call non-idempotent methods using GET request', async () => { const params = encodeURIComponent(JSON.stringify([1, 2])) await request(createApp()) - .get(`/myService?jsonrpc=2.0&id=1&method=add¶ms=${params}`) + .get(`/app/myService?jsonrpc=2.0&id=1&method=add¶ms=${params}`) .expect(405) }) @@ -269,9 +370,8 @@ describe('jsonrpc', () => { userId = 1000 const app = express() const myService = new MyService(5) - // console.log('service', myService, Object. app.use(bodyParser.json()) - app.use('/', + app.use('/app', jsonrpc( () => Promise.resolve({userId}), noopLogger, @@ -296,7 +396,7 @@ describe('jsonrpc', () => { params: [1, 2], } const response = await request(create()) - .post('/myService') + .post('/app/myService') .send(req) expect(response.body.result).toEqual(3) diff --git a/packages/jsonrpc/src/express.ts b/packages/jsonrpc/src/express.ts index b4acd0f..e0f38af 100644 --- a/packages/jsonrpc/src/express.ts +++ b/packages/jsonrpc/src/express.ts @@ -1,5 +1,5 @@ import { Logger } from '@rondo.dev/logger' -import express, { ErrorRequestHandler, Request, Response, Router } from 'express' +import express, { ErrorRequestHandler, Request, Response, NextFunction, RequestHandler } from 'express' import { createError, ErrorResponse, isRPCError } from './error' import { IDEMPOTENT_METHOD_REGEX } from './idempotent' import { createRpcService, ERROR_METHOD_NOT_FOUND, ERROR_SERVER, Request as RPCRequest, SuccessResponse } from './jsonrpc' @@ -13,7 +13,7 @@ export interface RPCReturnType { service: T, methods?: F[], ): RPCReturnType - router(): Router + router(): Array } export interface InvocationDetails { @@ -39,33 +39,7 @@ export function jsonrpc( idempotentMethodRegex = IDEMPOTENT_METHOD_REGEX, ): RPCReturnType { - /* eslint @typescript-eslint/no-unused-vars: 0 */ - const handleError: ErrorRequestHandler = (err, req, res, next) => { - logger.error('JSON-RPC Error: %s', err.stack) - - if (isRPCError(err)) { - res.status(err.statusCode) - res.json(err.response) - return - } - - const id = getRequestId(req) - const statusCode: number = err.statusCode || 500 - const errorResponse: ErrorResponse = { - jsonrpc: '2.0', - id, - result: null, - error: { - code: ERROR_SERVER.code, - message: statusCode >= 500 ? ERROR_SERVER.message : err.message, - data: 'errors' in err ? err.errors : null, - }, - } - res.status(statusCode) - res.json(errorResponse) - } - - const router = Router() + const router: Array = [] const self = { /** @@ -79,6 +53,36 @@ export function jsonrpc( ) { const rpcService = createRpcService(service, methods) + const handleError: ErrorRequestHandler = (err, req, res, next) => { + if (req.path !== path) { + next(err) + return + } + + logger.error('JSON-RPC Error: %s', err.stack) + + if (isRPCError(err)) { + res.status(err.statusCode) + res.json(err.response) + return + } + + const id = getRequestId(req) + const statusCode: number = err.statusCode || 500 + const errorResponse: ErrorResponse = { + jsonrpc: '2.0', + id, + result: null, + error: { + code: ERROR_SERVER.code, + message: statusCode >= 500 ? ERROR_SERVER.message : err.message, + data: 'errors' in err ? err.errors : null, + }, + } + res.status(statusCode) + res.json(errorResponse) + } + function handleResponse( response: SuccessResponse | null, res: Response, @@ -91,7 +95,25 @@ export function jsonrpc( } } - router.get(path, (req, res, next) => { + function handle(req: Request, res: Response, next: NextFunction) { + if (req.path !== path) { + next() + return + } + + switch(req.method) { + case 'GET': + handleGet(req, res, next) + break + case 'POST': + handlePost(req, res, next) + break + default: + next() + } + } + + const handleGet: RequestHandler = (req, res, next) => { if (!idempotentMethodRegex.test(req.query.method)) { // TODO fix status code and error type const err = createError(ERROR_METHOD_NOT_FOUND, { @@ -114,9 +136,9 @@ export function jsonrpc( (body = request) => rpcService.invoke(body, context))) .then(response => handleResponse(response, res)) .catch(next) - }) + } - router.post(path, (req, res, next) => { + const handlePost: RequestHandler = (req, res, next) => { Promise.resolve(getContext(req)) .then(context => hook( @@ -124,9 +146,10 @@ export function jsonrpc( (body = req.body) => rpcService.invoke(body, context))) .then(response => handleResponse(response, res)) .catch(next) - }) + } - router.use(path, handleError) + router.push(handle) + router.push(handleError) return self }, router() {