Skip to content
This repository has been archived by the owner on Dec 16, 2023. It is now read-only.

Replay does not return any data if stream is not immediately read #139

Open
nicolasazrak opened this issue Nov 10, 2017 · 3 comments
Open

Comments

@nicolasazrak
Copy link

I was trying to use https://github.com/sindresorhus/got with replay without success. When using replay the responses bodies are always empty. After reading the code of both libraries I discovered that replay returns a response and sets its stream in flowing state (https://nodejs.org/api/stream.html#stream_two_modes). The problem with that mode is that if the content is not immediately read, it will be lost. Libraries using http module expect the stream not to be flowing. This example reproduces the issue:

const replay = require('replay');
const http = require('http');

const req = http.request('http://dev.flowics.com:3000/', res => {
  setImmediate(() => {
    res.setEncoding('utf8');
    res.on('data', chunk => {
      console.log(`${chunk}`);
    });
    res.on('end', () => {
      console.log('No more data in response.');
    });
  });
});

req.end();

Without the setImmediate it works as expected. But it's different to how the http module works. Just removing this line solves the issue.

@jeromegn
Copy link
Contributor

jeromegn commented Dec 9, 2017

I've also encountered this issue just now. I'm keeping the http.IncomingMessage for a little while and only reading from it later (only if necessary). Without node-replay this is fine, but when using it the body is empty.

I'm wondering if changing it to not resume would break current usage or not. The current test suite fails on my machine. A few more tests fail when removing that response.resume() line, so I assume there would be breakage.

@zebulonj
Copy link
Contributor

zebulonj commented Mar 8, 2019

This is biting me too. Thanks for tracking down the cause @nicolasazrak.

@zebulonj
Copy link
Contributor

zebulonj commented Mar 8, 2019

@assaf Is there a reason that Replay is defaulting returned streams to flowing mode, when the default for Node is paused mode? I recognize that it would be a breaking change, but... it seems reasonable that when patching the http library, it's defaults would be preserved.

If you're not inclined to change that default behavior in Replay, would you be open to a pull-request that adds an option to disable the automatic flow of response data? I'd propose defaulting to automatic resume on capture (so it's not a breaking change), but allow users to restore the http default using an interface something like Replay.autoResume = false; (I propose setting that on the Replay object because that's how other options are currently set, i.e., Replay.mode.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants