Skip to content
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.

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

refactor(http): eagerly loaded HttpOutputEffect and HttpErrorEffect #357

Merged
merged 4 commits into from
Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions packages/http/src/+internal/testing.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,22 @@ export const createMockEffectContext = () => {
return createEffectContext({ ask: lookup(context), client });
};

export const createTestRoute = (opts?: { throwError?: boolean; delay?: number; method?: HttpMethod }) => {
export const createTestRoute = (opts?: { throwError?: boolean; delay?: number; method?: HttpMethod; effectSpy?: jest.Mock }) => {
const method = opts?.method ?? 'GET';
const routeDelay = opts?.delay ?? 0;

const req = createHttpRequest(({ url: `/delay_${routeDelay}`, method }));
const path = factorizeRegExpWithParams(`/delay_${routeDelay}`);

const effect: HttpEffect = req$ =>
req$.pipe(
const effect: HttpEffect = req$ => {
opts?.effectSpy?.();

return req$.pipe(
delay(routeDelay),
tap(() => { if (opts?.throwError) throw new Error(); }),
mapTo({ body: `delay_${routeDelay}` }),
);
};

const item: RoutingItem = {
regExp: path.regExp,
Expand Down
14 changes: 10 additions & 4 deletions packages/http/src/effects/http.effects.interface.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Event, Effect } from '@marblejs/core';
import { HttpRequest, HttpStatus, HttpHeaders, HttpServer } from '../http.interface';
import { HttpRequest, HttpStatus, HttpHeaders, HttpServer, WithHttpRequest } from '../http.interface';

export interface HttpEffectResponse<T = any> {
request?: HttpRequest;
Expand All @@ -15,16 +15,22 @@ export interface HttpMiddlewareEffect<

export interface HttpErrorEffect<
Err extends Error = Error,
> extends HttpEffect<{ req: HttpRequest; error: Err }, HttpEffectResponse> {}
Req extends HttpRequest = HttpRequest,
> extends HttpEffect<
WithHttpRequest<{ error: Err }, Req>,
WithHttpRequest<HttpEffectResponse>
> {}

export interface HttpServerEffect<
Ev extends Event = Event
> extends HttpEffect<Ev, any> {}

export interface HttpOutputEffect<
Req extends HttpRequest = HttpRequest,
Res extends HttpEffectResponse = HttpEffectResponse
> extends HttpEffect<{ req: Req; res: Res }, HttpEffectResponse> {}
> extends HttpEffect<
WithHttpRequest<HttpEffectResponse, Req>,
WithHttpRequest<HttpEffectResponse>
> {}

export interface HttpEffect<
I = HttpRequest,
Expand Down
14 changes: 7 additions & 7 deletions packages/http/src/effects/http.requestMetadata.effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import { HttpRequestMetadataStorageToken } from '../server/internal-dependencies
import { getHttpRequestMetadataIdHeader } from '../+internal/metadata.util';
import { HttpOutputEffect } from './http.effects.interface';

export const requestMetadata$: HttpOutputEffect = (out$, ctx) => {
export const requestMetadata$: HttpOutputEffect = (output$, ctx) => {
const httpRequestMetadataStorage = useContext(HttpRequestMetadataStorageToken)(ctx.ask);

return out$.pipe(
map(({ req, res }) => pipe(
getHttpRequestMetadataIdHeader(req.headers),
O.fold(constant(res), requestId => {
httpRequestMetadataStorage.set(requestId, req.meta);
return res;
return output$.pipe(
map(response => pipe(
getHttpRequestMetadataIdHeader(response.request.headers),
O.fold(constant(response), requestId => {
httpRequestMetadataStorage.set(requestId, response.request.meta);
return response;
}),
)),
);
Expand Down
17 changes: 7 additions & 10 deletions packages/http/src/error/http.error.effect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@ import { defaultError$ } from './http.error.effect';
import { HttpError } from './http.error.model';

describe('defaultError$', () => {
const req = createHttpRequest();
const request = createHttpRequest();
const client = createHttpResponse();
const ctx = { client };

test('maps HttpError', () => {
const error = new HttpError('test-message', 400);
const incomingRequest = { req, error };
const outgoingResponse = {
status: 400,
body: { error: {
Expand All @@ -20,14 +19,13 @@ describe('defaultError$', () => {
};

Marbles.assertEffect(defaultError$, [
['-a-', { a: incomingRequest }],
['-a-', { a: outgoingResponse }],
['-a-', { a: { request, error } }],
['-a-', { a: { request, ...outgoingResponse } }],
], { ctx });
});

test('maps other errors', () => {
const error = new Error('test-message');
const incomingRequest = { req, error };
const outgoingResponse = {
status: 500,
body: { error: {
Expand All @@ -37,14 +35,13 @@ describe('defaultError$', () => {
};

Marbles.assertEffect(defaultError$, [
['-a-', { a: incomingRequest }],
['-a-', { a: outgoingResponse }],
['-a-', { a: { request, error } }],
['-a-', { a: { request, ...outgoingResponse } }],
], { ctx });
});

test('maps to "Internal server error" if "error" is not provided', () => {
const error = undefined;
const incomingRequest = { req, error };
const outgoingResponse = {
status: 500,
body: { error: {
Expand All @@ -54,8 +51,8 @@ describe('defaultError$', () => {
};

Marbles.assertEffect(defaultError$, [
['-a-', { a: incomingRequest }],
['-a-', { a: outgoingResponse }],
['-a-', { a: { request, error } }],
['-a-', { a: { request, ...outgoingResponse } }],
], { ctx });
});
});
4 changes: 2 additions & 2 deletions packages/http/src/error/http.error.effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ const errorFactory = (status: HttpStatus) => (error: Error): HttpErrorResponse =

export const defaultError$: HttpErrorEffect = req$ =>
req$.pipe(
map(({ error = defaultHttpError }) => {
map(({ request, error = defaultHttpError }) => {
const status = getStatusCode(error);
const body = errorFactory(status)(error);
return { status, body };
return ({ status, body, request });
}),
);
9 changes: 2 additions & 7 deletions packages/http/src/error/http.error.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,8 @@ export const isHttpError = (error: Error | undefined): error is HttpError =>
export const isHttpRequestError = (error: Error | undefined): error is HttpRequestError =>
error?.name === HttpErrorType.HTTP_REQUEST_ERROR;

export const unexpectedErrorWhileSendingErrorFactory = (error: Error): CoreError => {
const message = `An unexpected error ${chalk.red(`"${error.message}"`)} occured while sending an error response. Please check your error effect.`;
return coreErrorFactory(message, { printStacktrace: false });
};

export const unexpectedErrorWhileSendingOutputFactory = (error: Error): CoreError => {
const message = `An unexpected error ${chalk.red(`"${error.message}"`)} occured while sending a response. Please check your output effect.`;
export const unexpectedErrorWhileSendingResponseFactory = (error: Error): CoreError => {
const message = `An unexpected error ${chalk.red(`"${error.message}"`)} occured while sending a response. Please check your output/error effect.`;
return coreErrorFactory(message, { printStacktrace: false });
};

Expand Down
10 changes: 10 additions & 0 deletions packages/http/src/http.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export interface HttpRequest<
[key: string]: any;
}

export type WithHttpRequest<T, Req extends HttpRequest = HttpRequest> =
{ request: Req } & T;

export interface RouteParameters {
[key: string]: any;
}
Expand All @@ -29,6 +32,13 @@ export interface QueryParameters {
}

export interface HttpResponse extends http.ServerResponse {
/**
* Send HTTP response
*
* @param response `HttpEffectResponse`
* @returns `Observable<boolean>` (indicates whether the response was sent or not)
* @since 1.0.0
*/
send: (response: HttpEffectResponse) => Observable<boolean>;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/http/src/router/http.router.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const decorateEffect = (stream: Observable<HttpRequest>, errorSubject: Er
...operations,
map((res: HttpEffectResponse) => ({ ...res, request })),
catchError((error: Error) => {
errorSubject.next({ error, req: request });
errorSubject.next({ error, request });
return EMPTY;
}),
])(of(request)) as Observable<HttpEffectResponse>),
Expand All @@ -32,7 +32,7 @@ export const decorateMiddleware = (stream: Observable<HttpRequest>, errorSubject
mergeMap((request: HttpRequest) => pipeFromArray([
...operations,
catchError((error: Error) => {
errorSubject.next({ error, req: request });
errorSubject.next({ error, request });
return EMPTY;
}),
])(of(request)) as Observable<HttpRequest>),
Expand Down
7 changes: 2 additions & 5 deletions packages/http/src/router/http.router.interface.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { Subject } from 'rxjs';
import { HttpMethod, HttpRequest } from '../http.interface';
import { HttpMethod, HttpRequest, WithHttpRequest } from '../http.interface';
import { HttpEffect, HttpMiddlewareEffect, HttpEffectResponse } from '../effects/http.effects.interface';

export type ErrorSubject = Subject<{
error: Error;
req: HttpRequest<unknown, unknown, unknown>;
}>;
export type ErrorSubject = Subject<WithHttpRequest<{ error: Error }>>;

// Route
export interface RouteMeta extends Record<string, any> {
Expand Down
Loading