Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

`test-form.js` - `handle ServerRequest POST` fails on node v0.10.0 #59

Closed
tellnes opened this Issue Mar 18, 2013 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

tellnes commented Mar 18, 2013

The test handle ServerRequest POST in test-form.js fails on node v0.10.0 with the following error:

TypeError: Cannot read property 'readable' of undefined
    at IncomingMessage._read (http.js:345:19)
    at IncomingMessage.Readable.read (_stream_readable.js:293:10)
    at IncomingMessage.read (http.js:337:15)
    at IncomingMessage.<anonymous> (_stream_readable.js:698:45)
    at IncomingMessage.EventEmitter.emit (events.js:92:17)
    at emitDataEvents (_stream_readable.js:724:10)
    at IncomingMessage.Readable.on (_stream_readable.js:656:5)
    at Object.f.handle (/Users/christian/code/forms/lib/forms.js:63:29)
    at Object.exports.handle ServerRequest POST (/Users/christian/code/forms/test/test-form.js:276:7)
    at Object.<anonymous> (/Users/christian/code/forms/node_modules/nodeunit/lib/core.js:235:16)

The problem seems to be that http.IncomingMessage requires a socket as the first parameter.

Collaborator

ljharb commented Mar 19, 2013

Actually this is related to Streams2 changes in node http://blog.nodejs.org/2012/12/20/streams2/ - the socket param would break more than one test if that was the problem. I'm looking into this.

@ljharb ljharb was assigned Mar 19, 2013

Contributor

tellnes commented Mar 20, 2013

I actually tried to fix it by sending new net.Socket() to http.IncomingMessage, it did fix it, but it broke some other tests in a strange way.

I think it should be possible to pass inn use any readable stream. Eg. union uses a custom request object which forms will fail to recognize.

Maybe we just should change form.handle to do (obj instanceof stream.Readable || obj instanceof http.IncomingMessage).

Collaborator

ljharb commented Apr 30, 2013

Fixed with #62

@ljharb ljharb closed this Apr 30, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment