Skip to content

Commit

Permalink
fix: error serialisation SO-195 (#274)
Browse files Browse the repository at this point in the history
* fix: manually serialise the payload

* refactor: avoid cast

* test: check all the error responses

* fix: shuold always be a correct shape

* refactor: move things in the ProblemJson handler

* fix: problem/json property
  • Loading branch information
XVincentX authored May 9, 2019
1 parent a17c19d commit 1199919
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 23 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/utils/graphFacade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class GraphFacade {
language,
path: '/',
data: { raw },
} as ISourceNode);
});

await this.graphite.scheduler.drain();
}
Expand Down
16 changes: 13 additions & 3 deletions packages/http-server/src/__tests__/server.oas.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ import { relative, resolve } from 'path';
import { createServer } from '../';
import { IPrismHttpServer } from '../types';

function checkErrorPayloadShape(payload: string) {
const parsedPayload = JSON.parse(payload);

expect(parsedPayload).toHaveProperty('type');
expect(parsedPayload).toHaveProperty('title');
expect(parsedPayload).toHaveProperty('status');
expect(parsedPayload).toHaveProperty('detail');
}

describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', file => {
let server: IPrismHttpServer<{}>;

Expand Down Expand Up @@ -37,6 +46,7 @@ describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', fil
url: '/pets/123',
});
expect(response.statusCode).toBe(500);
checkErrorPayloadShape(response.payload);
});

test('will return requested response using the __code property', async () => {
Expand Down Expand Up @@ -68,6 +78,7 @@ describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', fil
});

expect(response.statusCode).toBe(422);
checkErrorPayloadShape(response.payload);
});

test.skip('should automagically provide the parameters when not provided in the query string and a default is defined', async () => {
Expand Down Expand Up @@ -137,6 +148,7 @@ describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', fil
},
});
expect(response.statusCode).toBe(422);
checkErrorPayloadShape(response.payload);
});

test('will return the default response when using the __code property with a non existing code', async () => {
Expand All @@ -146,9 +158,6 @@ describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', fil
});

expect(response.statusCode).toBe(499);
const payload = JSON.parse(response.payload);
expect(payload).toHaveProperty('code');
expect(payload).toHaveProperty('message');
});

test('will return 500 with error when an undefined code is requested and there is no default response', async () => {
Expand All @@ -158,6 +167,7 @@ describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', fil
});

expect(response.statusCode).toBe(500);
checkErrorPayloadShape(response.payload);
});
});

Expand Down
13 changes: 2 additions & 11 deletions packages/http-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,9 @@ const replyHandler = <LoaderInput>(
const status = 'status' in e ? e.status : 500;
reply
.type('application/problem+json')
.serializer((payload: unknown) => JSON.stringify(payload))
.serializer(JSON.stringify)
.code(status)
.send(
ProblemJsonError.fromTemplate(
{
name: e.name || 'UNKNOWN',
title: e.message,
status,
},
e.detail,
),
);
.send(ProblemJsonError.fromPlainError(e));
}
};
};
2 changes: 1 addition & 1 deletion packages/http/src/mocker/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Omit } from '@stoplight/types';
import { ProblemJson } from '../types';

export const UNPROCESSABLE_ENTITY: Omit<ProblemJson, 'detail'> = {
name: 'UNPROCESSABLE_ENTITY',
type: 'UNPROCESSABLE_ENTITY',
title: 'Invalid request body payload',
status: 422,
};
10 changes: 5 additions & 5 deletions packages/http/src/router/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@ import { ProblemJson } from '../types';

export const NO_RESOURCE_PROVIDED_ERROR: Omit<ProblemJson, 'detail'> = {
title: 'Route not resolved, no resource provided.',
name: 'NO_RESOURCE_PROVIDED_ERROR',
type: 'NO_RESOURCE_PROVIDED_ERROR',
status: 404,
};
export const NO_PATH_MATCHED_ERROR: Omit<ProblemJson, 'detail'> = {
title: 'Route not resolved, no path matched.',
name: 'NO_PATH_MATCHED_ERROR',
type: 'NO_PATH_MATCHED_ERROR',
status: 404,
};
export const NO_SERVER_MATCHED_ERROR: Omit<ProblemJson, 'detail'> = {
title: 'Route not resolved, no server matched.',
name: 'NO_SERVER_MATCHED_ERROR',
type: 'NO_SERVER_MATCHED_ERROR',
status: 404,
};
export const NO_METHOD_MATCHED_ERROR: Omit<ProblemJson, 'detail'> = {
title: 'Route resolved, but no path matched.',
name: 'NO_METHOD_MATCHED_ERROR',
type: 'NO_METHOD_MATCHED_ERROR',
status: 405,
};
export const NO_SERVER_CONFIGURATION_PROVIDED_ERROR: Omit<ProblemJson, 'detail'> = {
title: 'Route not resolved, no server configuration provided.',
name: 'NO_SERVER_CONFIGURATION_PROVIDED_ERROR',
type: 'NO_SERVER_CONFIGURATION_PROVIDED_ERROR',
status: 404,
};
13 changes: 11 additions & 2 deletions packages/http/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export interface IHttpResponse {
}

export type ProblemJson = {
name: string;
type: string;
title: string;
status: number;
detail: string;
Expand All @@ -86,13 +86,22 @@ export type ProblemJson = {
export class ProblemJsonError extends Error {
public static fromTemplate(template: Omit<ProblemJson, 'detail'>, detail?: string): ProblemJsonError {
return new ProblemJsonError(
`https://stoplight.io/prism/errors#${template.name}`,
`https://stoplight.io/prism/errors#${template.type}`,
template.title,
template.status,
detail || '',
);
}

public static fromPlainError(error: Error & { detail?: string; status?: number }): ProblemJson {
return {
type: error.name && error.name !== 'Error' ? error.name : 'https://stoplight.io/prism/errors#UNKNOWN',
title: error.message,
status: error.status || 500,
detail: error.detail || '',
};
}

constructor(readonly name: string, readonly message: string, readonly status: number, readonly detail: string) {
super(message);
Error.captureStackTrace(this, ProblemJsonError);
Expand Down

0 comments on commit 1199919

Please sign in to comment.