Skip to content

Commit

Permalink
fix: Prevent warnings for expected errors
Browse files Browse the repository at this point in the history
  • Loading branch information
joachimvh committed Jul 29, 2021
1 parent a926839 commit b1b4b2b
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 3 deletions.
10 changes: 9 additions & 1 deletion src/util/StreamUtil.ts
Expand Up @@ -23,6 +23,12 @@ export async function readableToString(stream: Readable): Promise<string> {
return (await arrayifyStream(stream)).join('');
}

// These error messages usually indicate expected behaviour so should not give a warning.
const safeErrors = new Set([
'Cannot call write after a stream was destroyed',
'premature close',
]);

/**
* Pipes one stream into another and emits errors of the first stream with the second.
* In case of an error in the first stream the second one will be destroyed with the given error.
Expand Down Expand Up @@ -53,7 +59,9 @@ export function pipeSafely<T extends Writable>(readable: NodeJS.ReadableStream,
// In case the input readable is guarded, it will no longer log errors since `pump` attaches a new error listener
pump(readable, destination, (error): void => {
if (error) {
logger.warn(`Piped stream errored with ${error.message}`);
const msg = `Piped stream errored with ${error.message}`;
logger.log(safeErrors.has(error.message) ? 'debug' : 'warn', msg);

// Make sure the final error can be handled in a normal streaming fashion
destination.emit('error', mapError ? mapError(error) : error);
}
Expand Down
3 changes: 2 additions & 1 deletion test/integration/GuardedStream.test.ts
Expand Up @@ -14,7 +14,8 @@ import type { Representation,
Logger } from '../../src';

jest.mock('../../src/logging/LogUtil', (): any => {
const logger: Logger = { error: jest.fn(), debug: jest.fn(), warn: jest.fn(), info: jest.fn() } as any;
const logger: Logger =
{ error: jest.fn(), debug: jest.fn(), warn: jest.fn(), info: jest.fn(), log: jest.fn() } as any;
return { getLoggerFor: (): Logger => logger };
});
const logger: jest.Mocked<Logger> = getLoggerFor('GuardedStream') as any;
Expand Down
28 changes: 27 additions & 1 deletion test/unit/util/StreamUtil.test.ts
@@ -1,9 +1,17 @@
import { PassThrough } from 'stream';
import arrayifyStream from 'arrayify-stream';
import streamifyArray from 'streamify-array';
import type { Logger } from '../../../src/logging/Logger';
import { getLoggerFor } from '../../../src/logging/LogUtil';
import { isHttpRequest } from '../../../src/server/HttpRequest';
import { guardedStreamFrom, pipeSafely, transformSafely, readableToString } from '../../../src/util/StreamUtil';

jest.mock('../../../src/logging/LogUtil', (): any => {
const logger: Logger = { warn: jest.fn(), log: jest.fn() } as any;
return { getLoggerFor: (): Logger => logger };
});
const logger: jest.Mocked<Logger> = getLoggerFor('StreamUtil') as any;

jest.mock('../../../src/server/HttpRequest', (): any => ({
isHttpRequest: jest.fn(),
}));
Expand All @@ -18,7 +26,7 @@ describe('StreamUtil', (): void => {

describe('#pipeSafely', (): void => {
beforeEach(async(): Promise<void> => {
(isHttpRequest as unknown as jest.Mock).mockClear();
jest.clearAllMocks();
});

it('pipes data from one stream to the other.', async(): Promise<void> => {
Expand All @@ -37,6 +45,8 @@ describe('StreamUtil', (): void => {
const output = new PassThrough();
const piped = pipeSafely(input, output);
await expect(readableToString(piped)).rejects.toThrow('error');
expect(logger.log).toHaveBeenCalledTimes(1);
expect(logger.log).toHaveBeenLastCalledWith('warn', 'Piped stream errored with error');
});

it('supports mapping errors to something else.', async(): Promise<void> => {
Expand All @@ -50,6 +60,22 @@ describe('StreamUtil', (): void => {
await expect(readableToString(piped)).rejects.toThrow('other error');
});

it('logs specific safer errors as debug.', async(): Promise<void> => {
const input = streamifyArray([ 'data' ]);
input.read = (): any => {
input.emit('error', new Error('Cannot call write after a stream was destroyed'));
return null;
};
const output = new PassThrough();
const piped = pipeSafely(input, output);
await expect(readableToString(piped)).rejects.toThrow('Cannot call write after a stream was destroyed');
expect(logger.log).toHaveBeenCalledTimes(1);
expect(logger.log).toHaveBeenLastCalledWith(
'debug',
'Piped stream errored with Cannot call write after a stream was destroyed',
);
});

it('destroys the source stream in case the destinations becomes unpiped.', async(): Promise<void> => {
const input = new PassThrough();
const output = new PassThrough();
Expand Down

0 comments on commit b1b4b2b

Please sign in to comment.