Skip to content

change the way response is used #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2022
Merged

change the way response is used #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e191cf
feat: factor response library out of io-ts-http
bitgopatmcl Apr 26, 2022
7dd2fff
feat: handle status codes or response keys in express-wrapper
bitgopatmcl Apr 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 14 additions & 112 deletions packages/express-wrapper/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,122 +4,25 @@
*/

import express from 'express';
import * as PathReporter from 'io-ts/lib/PathReporter';

import {
ApiSpec,
HttpResponseCodes,
HttpRoute,
RequestType,
ResponseType,
} from '@api-ts/io-ts-http';
import { ApiSpec, HttpRoute } from '@api-ts/io-ts-http';

import { apiTsPathToExpress } from './path';

export type Function<R extends HttpRoute> = (
input: RequestType<R>,
) => ResponseType<R> | Promise<ResponseType<R>>;
export type RouteStack<R extends HttpRoute> = [
...express.RequestHandler[],
Function<R>,
];

/**
* Dynamically assign a function name to avoid anonymous functions in stack traces
* https://stackoverflow.com/a/69465672
*/
const createNamedFunction = <F extends (...args: any) => void>(
name: string,
fn: F,
): F => Object.defineProperty(fn, 'name', { value: name });

const isKnownStatusCode = (code: string): code is keyof typeof HttpResponseCodes =>
HttpResponseCodes.hasOwnProperty(code);

const decodeRequestAndEncodeResponse = <Route extends HttpRoute>(
apiName: string,
httpRoute: Route,
handler: Function<Route>,
): express.RequestHandler => {
return createNamedFunction(
'decodeRequestAndEncodeResponse' + httpRoute.method + apiName,
async (req, res) => {
const maybeRequest = httpRoute.request.decode(req);
if (maybeRequest._tag === 'Left') {
console.log('Request failed to decode');
const validationErrors = PathReporter.failure(maybeRequest.left);
const validationErrorMessage = validationErrors.join('\n');
res.writeHead(400, { 'Content-Type': 'application/json' });
res.write(JSON.stringify({ error: validationErrorMessage }));
res.end();
return;
}

let rawResponse: ResponseType<Route> | undefined;
try {
rawResponse = await handler(maybeRequest.right);
} catch (err) {
console.warn('Error in route handler:', err);
res.statusCode = 500;
res.end();
return;
}

// Take the first match -- the implication is that the ordering of declared response
// codecs is significant!
for (const [statusCode, responseCodec] of Object.entries(httpRoute.response)) {
if (rawResponse.type !== statusCode) {
continue;
}

if (!isKnownStatusCode(statusCode)) {
console.warn(
`Got unrecognized status code ${statusCode} for ${apiName} ${httpRoute.method}`,
);
res.status(500);
res.end();
return;
}

// We expect that some route implementations may "beat the type
// system away with a stick" and return some unexpected values
// that fail to encode, so we catch errors here just in case
let response: unknown;
try {
response = responseCodec.encode(rawResponse.payload);
} catch (err) {
console.warn(
"Unable to encode route's return value, did you return the expected type?",
err,
);
res.statusCode = 500;
res.end();
return;
}
// DISCUSS: safer ways to handle this cast
res.writeHead(HttpResponseCodes[statusCode], {
'Content-Type': 'application/json',
});
res.write(JSON.stringify(response));
res.end();
return;
}

// If we got here then we got an unexpected response
res.status(500);
res.end();
},
);
};
import {
decodeRequestAndEncodeResponse,
getMiddleware,
getServiceFunction,
RouteHandler,
} from './request';

const isHttpVerb = (verb: string): verb is 'get' | 'put' | 'post' | 'delete' =>
({ get: 1, put: 1, post: 1, delete: 1 }.hasOwnProperty(verb));
verb === 'get' || verb === 'put' || verb === 'post' || verb === 'delete';

export function createServer<Spec extends ApiSpec>(
spec: Spec,
configureExpressApplication: (app: express.Application) => {
[ApiName in keyof Spec]: {
[Method in keyof Spec[ApiName]]: RouteStack<Spec[ApiName][Method]>;
[Method in keyof Spec[ApiName]]: RouteHandler<Spec[ApiName][Method]>;
};
},
) {
Expand All @@ -134,14 +37,13 @@ export function createServer<Spec extends ApiSpec>(
continue;
}
const httpRoute: HttpRoute = resource[method]!;
const stack = routes[apiName]![method]!;
// Note: `stack` is guaranteed to be non-empty thanks to our function's type signature
const handler = decodeRequestAndEncodeResponse(
const routeHandler = routes[apiName]![method]!;
const expressRouteHandler = decodeRequestAndEncodeResponse(
apiName,
httpRoute,
stack[stack.length - 1] as Function<HttpRoute>,
httpRoute as any, // TODO: wat
getServiceFunction(routeHandler),
);
const handlers = [...stack.slice(0, stack.length - 1), handler];
const handlers = [...getMiddleware(routeHandler), expressRouteHandler];

const expressPath = apiTsPathToExpress(httpRoute.path);
router[method](expressPath, handlers);
Expand Down
103 changes: 103 additions & 0 deletions packages/express-wrapper/src/request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* express-wrapper
* A simple, type-safe web server
*/

import express from 'express';
import * as t from 'io-ts';
import * as PathReporter from 'io-ts/lib/PathReporter';

import {
HttpRoute,
HttpToKeyStatus,
KeyToHttpStatus,
RequestType,
ResponseType,
} from '@api-ts/io-ts-http';

type NumericOrKeyedResponseType<R extends HttpRoute> =
| ResponseType<R>
| {
[S in keyof R['response']]: S extends keyof HttpToKeyStatus
? {
type: HttpToKeyStatus[S];
payload: t.TypeOf<R['response'][S]>;
}
: never;
}[keyof R['response']];

export type ServiceFunction<R extends HttpRoute> = (
input: RequestType<R>,
) => NumericOrKeyedResponseType<R> | Promise<NumericOrKeyedResponseType<R>>;

export type RouteHandler<R extends HttpRoute> =
| ServiceFunction<R>
| { middleware: express.RequestHandler[]; handler: ServiceFunction<R> };

export const getServiceFunction = <R extends HttpRoute>(
routeHandler: RouteHandler<R>,
): ServiceFunction<R> =>
'handler' in routeHandler ? routeHandler.handler : routeHandler;

export const getMiddleware = <R extends HttpRoute>(
routeHandler: RouteHandler<R>,
): express.RequestHandler[] =>
'middleware' in routeHandler ? routeHandler.middleware : [];

/**
* Dynamically assign a function name to avoid anonymous functions in stack traces
* https://stackoverflow.com/a/69465672
*/
const createNamedFunction = <F extends (...args: any) => void>(
name: string,
fn: F,
): F => Object.defineProperty(fn, 'name', { value: name });

export const decodeRequestAndEncodeResponse = <Route extends HttpRoute>(
apiName: string,
httpRoute: Route,
handler: ServiceFunction<Route>,
): express.RequestHandler => {
return createNamedFunction(
'decodeRequestAndEncodeResponse' + httpRoute.method + apiName,
async (req, res) => {
const maybeRequest = httpRoute.request.decode(req);
if (maybeRequest._tag === 'Left') {
console.log('Request failed to decode');
Copy link

@ryan-bitgo ryan-bitgo Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will people be using this function? Cuz this would log in their console right which is something they might not want

Copy link
Contributor Author

@bitgopatmcl bitgopatmcl Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is intended to be package-internal. It's exported from this file but not re-exported from index. Does express have some kind of logging system? I don't see one skimming the docs.

Copy link
Contributor

@ericcrosson-bitgo ericcrosson-bitgo Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I ponder this, I will point out the 12-factor way is "log to stdout/stderr"

https://12factor.net/logs

Copy link

@ryan-bitgo ryan-bitgo Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok yeah its probably fine then

Copy link
Contributor

@ericcrosson-bitgo ericcrosson-bitgo May 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did come across this over the weekend, is it destiny? or inapplicable

image

https://www.lpalmieri.com/posts/error-handling-rust/

Here we are handling the error on the developer's behalf, and it would be transparent if not for this error. According to this advice, logging here is the proper action

const validationErrors = PathReporter.failure(maybeRequest.left);
const validationErrorMessage = validationErrors.join('\n');
res.writeHead(400, { 'Content-Type': 'application/json' });
res.write(JSON.stringify({ error: validationErrorMessage }));
res.end();
return;
}

let rawResponse: NumericOrKeyedResponseType<Route> | undefined;
try {
rawResponse = await handler(maybeRequest.right);
} catch (err) {
console.warn('Error in route handler:', err);
res.status(500).end();
return;
}

const { type, payload } = rawResponse;
const status = typeof type === 'number' ? type : (KeyToHttpStatus as any)[type];
if (status === undefined) {
console.warn('Unknown status code returned');
res.status(500).end();
return;
}
const responseCodec = httpRoute.response[status];
if (responseCodec === undefined || !responseCodec.is(payload)) {
console.warn(
"Unable to encode route's return value, did you return the expected type?",
);
res.status(500).end();
return;
}

res.status(status).json(responseCodec.encode(payload)).end();
},
);
};
Loading