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

npm install fails in res.resume() while working around bug in node v0.10.0 where the CryptoStream gets stuck and never starts reading again #10

Open
deathcap opened this issue Feb 10, 2015 · 2 comments

Comments

@deathcap
Copy link
Owner

After fixing GH-9, new issue with npm install:


npm.commands.install(['voxel-engine'])
bundle.js:76 npm command require /node_modules/npm/lib/install.js
undefined
res
module.exports {offset: 0, readable: true, _events: Object, statusCode: 200, headers: Object…}
res.resume
undefined
bundle.js:92892 Uncaught TypeError: undefined is not a function

in:

      req.on("response", function (res) {
        client.log.http("fetch", "" + res.statusCode, uri)

        var er
        var statusCode = res && res.statusCode
        if (statusCode === 200) {
          // Work around bug in node v0.10.0 where the CryptoStream
          // gets stuck and never starts reading again.
          res.resume()        // *** <-- res doesn't have a resume property
          if (process.version === "v0.10.0") unstick(res)

          return cb(null, res)
        }

res the the HTTP response from request, but when browserified it lacks .resume

https://github.com/substack/http-browserify

http://nodejs.org/api/stream.html#stream_readable_resume

@deathcap
Copy link
Owner Author

resume() is from Readable http://nodejs.org/api/stream.html , but http-browserify Response inherits from Stream https://github.com/substack/http-browserify/blob/5620574fd6606c606e9f4638ee0ab3685adc49e6/lib/response.js#L9:

util.inherits(Response, Stream);

node http IncomingMessage inherits from Readable not Stream: https://github.com/joyent/node/blob/1781c8b85bbabc4c5c1e054bd5c50903cc0eb47b/lib/_http_incoming.js#L80

util.inherits(IncomingMessage, Stream.Readable);

but if I change http-browserify to use Readable, crashes in Readable:

function emitDataEvents(stream, startPaused) {
  var state = stream._readableState;

  if (state.flowing) {   // <-- ** state is undefined
    // https://github.com/isaacs/readable-stream/issues/16
    throw new Error('Cannot switch to old mode now.');
  }

and the calling method in request has this comment:

Request.prototype.onRequestResponse = function (response) {

  } else if (response.resume) {
    // response.resume should be defined, but check anyway before calling. Workaround for browserify.
    response.resume()
  }

originally added in request/request@6ebd748 Add some additional hacks to work in the browser.

update: reported browserify/http-browserify#81

@deathcap
Copy link
Owner Author

Peeling the onion, with the latest workarounds npm.commands.install(['ucfirst']) is able to start downloading a package, but there's a problem in readable_stream./lib/_stream_writable.js:

function doWrite(stream, state, len, chunk, encoding, cb) {
  state.writelen = len;
  state.writecb = cb;
  state.writing = true;
  state.sync = true;
  stream._write(chunk, encoding, state.onwrite);
  state.sync = false;
}

stream is a WriteStream {__atomicTarget: "/tmp/npm-undefined-0ac88dd4/cors.maxogden.com/http_3A/registry.npmjs.org/ucfirst/-/ucfirst-0.0.1.tgz", __atomicChown: undefined, __atomicDidStuff: false, __atomicTmp: "/tmp/npm-undefined-0ac88dd4/cors.maxogden.com/http…cfirst-0.0.1.tgz.295b6409115f91279c312ebfded98842", _writableState: WritableState…}, but the ._write method is not overridden:

 function (chunk, encoding, cb) {
  cb(new Error('not implemented'));
}

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

No branches or pull requests

1 participant