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

Wait until body file descriptor is open #1054

Merged
merged 3 commits into from
Feb 6, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 12 additions & 6 deletions source/request-as-event-emitter.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {ReadStream} from 'fs';
Copy link
Owner

Choose a reason for hiding this comment

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

Generally, it's nicer to rename generic imports like this as it's not immediately clear further down in the code that it's an fs read stream.

import {ReadStream: FsReadStream} from 'fs';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do :)

import CacheableRequest = require('cacheable-request');
import EventEmitter = require('events');
import http = require('http');
Expand All @@ -15,6 +16,7 @@ import {createProgressStream} from './progress';
import timedOut, {TimeoutError as TimedOutTimeoutError} from './utils/timed-out';
import {GeneralError, NormalizedOptions, Response, requestSymbol} from './types';
import urlToOptions from './utils/url-to-options';
import pEvent = require('p-event');

const setImmediateAsync = async (): Promise<void> => new Promise(resolve => setImmediate(resolve));
const pipeline = promisify(stream.pipeline);
Expand Down Expand Up @@ -301,13 +303,17 @@ export default (options: NormalizedOptions): RequestAsEventEmitter => {
};

(async () => {
// Promises are executed immediately.
// If there were no `setImmediate` here,
// `promise.json()` would have no effect
// as the request would be sent already.
await setImmediateAsync();

try {
if (options.body instanceof ReadStream) {
await pEvent(options.body, 'open');
}

// Promises are executed immediately.
// If there were no `setImmediate` here,
// `promise.json()` would have no effect
// as the request would be sent already.
await setImmediateAsync();

for (const hook of options.hooks.beforeRequest) {
// eslint-disable-next-line no-await-in-loop
await hook(options);
Expand Down
29 changes: 29 additions & 0 deletions test/post.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import {promisify} from 'util';
import stream = require('stream');
import fs = require('fs');
import test from 'ava';
import delay = require('delay');
import {Handler} from 'express';
import getStream = require('get-stream');
import toReadableStream = require('to-readable-stream');
import got from '../source';
import withServer from './helpers/with-server';
Expand Down Expand Up @@ -271,3 +274,29 @@ test('DELETE method sends plain objects as JSON', withServer, async (t, server,
});
t.deepEqual(body, {such: 'wow'});
});

test('catches body errors before calling pipeline() - promise', withServer, async (t, server, got) => {
server.post('/', defaultEndpoint);

await t.throwsAsync(got.post({
body: fs.createReadStream('./file-that-does-not-exist.txt')
}), {
message: /ENOENT: no such file or directory/
});

// Wait for unhandled errors
await delay(100);
});

test('catches body errors before calling pipeline() - stream', withServer, async (t, server, got) => {
server.post('/', defaultEndpoint);

await t.throwsAsync(getStream(got.stream.post({
body: fs.createReadStream('./file-that-does-not-exist.txt')
})), {
message: /ENOENT: no such file or directory/
});

// Wait for unhandled errors
await delay(100);
});