mirror of
https://github.com/CommunitySolidServer/CommunitySolidServer.git
synced 2024-10-03 14:55:10 +00:00
fix: Do not write error if response already started.
This commit is contained in:
parent
1d77a28cd1
commit
907caa1e93
@ -28,7 +28,11 @@ export class ExpressHttpServerFactory implements HttpServerFactory {
|
|||||||
} catch (error: unknown) {
|
} catch (error: unknown) {
|
||||||
const errMsg = error instanceof Error ? `${error.name}: ${error.message}\n${error.stack}` : 'Unknown error.';
|
const errMsg = error instanceof Error ? `${error.name}: ${error.message}\n${error.stack}` : 'Unknown error.';
|
||||||
this.logger.error(errMsg);
|
this.logger.error(errMsg);
|
||||||
response.status(500).contentType('text/plain').send(errMsg);
|
if (response.headersSent) {
|
||||||
|
response.end();
|
||||||
|
} else {
|
||||||
|
response.status(500).contentType('text/plain').send(errMsg);
|
||||||
|
}
|
||||||
} finally {
|
} finally {
|
||||||
done();
|
done();
|
||||||
}
|
}
|
||||||
|
@ -1,59 +1,33 @@
|
|||||||
import type { Server } from 'http';
|
import type { Server } from 'http';
|
||||||
import request from 'supertest';
|
import request from 'supertest';
|
||||||
import { ExpressHttpServerFactory } from '../../../src/server/ExpressHttpServerFactory';
|
import { ExpressHttpServerFactory } from '../../../src/server/ExpressHttpServerFactory';
|
||||||
import { HttpHandler } from '../../../src/server/HttpHandler';
|
import type { HttpHandler } from '../../../src/server/HttpHandler';
|
||||||
import type { HttpRequest } from '../../../src/server/HttpRequest';
|
|
||||||
import type { HttpResponse } from '../../../src/server/HttpResponse';
|
import type { HttpResponse } from '../../../src/server/HttpResponse';
|
||||||
import SpyInstance = jest.SpyInstance;
|
|
||||||
|
|
||||||
const handle = async(input: { request: HttpRequest; response: HttpResponse }): Promise<void> => {
|
const handler: jest.Mocked<HttpHandler> = {
|
||||||
input.response.writeHead(200);
|
handleSafe: jest.fn(async(input: { response: HttpResponse }): Promise<void> => {
|
||||||
input.response.end();
|
input.response.writeHead(200);
|
||||||
};
|
input.response.end();
|
||||||
|
}),
|
||||||
class SimpleHttpHandler extends HttpHandler {
|
} as any;
|
||||||
public async handle(input: { request: HttpRequest; response: HttpResponse }): Promise<void> {
|
|
||||||
return handle(input);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
describe('ExpressHttpServerFactory', (): void => {
|
describe('ExpressHttpServerFactory', (): void => {
|
||||||
let server: Server;
|
let server: Server;
|
||||||
let canHandleJest: jest.Mock<Promise<void>, []>;
|
|
||||||
let handleJest: jest.Mock<Promise<void>, [any]>;
|
|
||||||
let handler: SimpleHttpHandler;
|
|
||||||
let mock: SpyInstance;
|
|
||||||
|
|
||||||
beforeAll(async(): Promise<void> => {
|
beforeAll(async(): Promise<void> => {
|
||||||
// Prevent test from writing to stderr
|
|
||||||
mock = jest.spyOn(process.stderr, 'write').mockImplementation((): boolean => true);
|
|
||||||
});
|
|
||||||
|
|
||||||
beforeEach(async(): Promise<void> => {
|
|
||||||
handler = new SimpleHttpHandler();
|
|
||||||
canHandleJest = jest.fn(async(): Promise<void> => undefined);
|
|
||||||
handleJest = jest.fn(async(input): Promise<void> => handle(input));
|
|
||||||
|
|
||||||
handler.canHandle = canHandleJest;
|
|
||||||
handler.handle = handleJest;
|
|
||||||
|
|
||||||
const factory = new ExpressHttpServerFactory(handler);
|
const factory = new ExpressHttpServerFactory(handler);
|
||||||
server = factory.startServer(5555);
|
server = factory.startServer(5555);
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(async(): Promise<void> => {
|
|
||||||
server.close();
|
|
||||||
});
|
|
||||||
|
|
||||||
afterAll(async(): Promise<void> => {
|
afterAll(async(): Promise<void> => {
|
||||||
mock.mockReset();
|
server.close();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('sends incoming requests to the handler.', async(): Promise<void> => {
|
it('sends incoming requests to the handler.', async(): Promise<void> => {
|
||||||
await request(server).get('/').set('Host', 'test.com').expect(200);
|
await request(server).get('/').set('Host', 'test.com').expect(200);
|
||||||
expect(canHandleJest).toHaveBeenCalledTimes(1);
|
|
||||||
expect(handleJest).toHaveBeenCalledTimes(1);
|
expect(handler.handleSafe).toHaveBeenCalledTimes(1);
|
||||||
expect(handleJest).toHaveBeenLastCalledWith({
|
expect(handler.handleSafe).toHaveBeenLastCalledWith({
|
||||||
request: expect.objectContaining({
|
request: expect.objectContaining({
|
||||||
headers: expect.objectContaining({ host: 'test.com' }),
|
headers: expect.objectContaining({ host: 'test.com' }),
|
||||||
}),
|
}),
|
||||||
@ -62,25 +36,31 @@ describe('ExpressHttpServerFactory', (): void => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('returns a 404 when the handler does not do anything.', async(): Promise<void> => {
|
it('returns a 404 when the handler does not do anything.', async(): Promise<void> => {
|
||||||
handler.handle = async(input): Promise<void> => {
|
handler.handleSafe.mockResolvedValueOnce(undefined);
|
||||||
expect(input).toBeDefined();
|
|
||||||
};
|
await expect(request(server).get('/').expect(404)).resolves.toBeDefined();
|
||||||
await request(server).get('/').expect(404);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('catches errors thrown by its handler.', async(): Promise<void> => {
|
it('writes an error to the HTTP response.', async(): Promise<void> => {
|
||||||
handler.handle = async(): Promise<void> => {
|
handler.handleSafe.mockRejectedValueOnce(new Error('dummyError'));
|
||||||
throw new Error('dummyError');
|
|
||||||
};
|
|
||||||
|
|
||||||
const res = await request(server).get('/').expect(500);
|
const res = await request(server).get('/').expect(500);
|
||||||
|
expect(res.headers['content-type']).toBe('text/plain; charset=utf-8');
|
||||||
expect(res.text).toContain('dummyError');
|
expect(res.text).toContain('dummyError');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not write an error if the response had been started.', async(): Promise<void> => {
|
||||||
|
handler.handleSafe.mockImplementationOnce(async(input: { response: HttpResponse }): Promise<void> => {
|
||||||
|
input.response.write('content');
|
||||||
|
throw new Error('dummyError');
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await request(server).get('/');
|
||||||
|
expect(res.text).not.toContain('dummyError');
|
||||||
|
});
|
||||||
|
|
||||||
it('throws unknown errors if its handler throw non-Error objects.', async(): Promise<void> => {
|
it('throws unknown errors if its handler throw non-Error objects.', async(): Promise<void> => {
|
||||||
handler.handle = async(): Promise<void> => {
|
handler.handleSafe.mockRejectedValueOnce('apple');
|
||||||
throw 'apple';
|
|
||||||
};
|
|
||||||
|
|
||||||
const res = await request(server).get('/').expect(500);
|
const res = await request(server).get('/').expect(500);
|
||||||
expect(res.text).toContain('Unknown error.');
|
expect(res.text).toContain('Unknown error.');
|
||||||
|
Loading…
x
Reference in New Issue
Block a user