mjackson / mach

HTTP for JavaScript
810 stars 41 forks source link

Errors upgrading to 1.0 #60

Closed natew closed 9 years ago

natew commented 9 years ago

Not sure why but my old version stopped working after a force restart. I decided to upgrade it and now have a couple issues.

First is when I try and add this:

var staticPaths = [ '/build/public', '/app/assets', '/web_modules' ];
staticPaths.forEach(function(path) {
  app.use(mach.file, { root: __dirname + path });
});

I get these errors on every route, even ones that it shouldn't match:

Error: Invalid root directory: undefined
    at file (/Users/nw/Sites/write/node_modules/mach/modules/file.js:77:11)
    at /Users/nw/Sites/write/node_modules/mach/modules/stack.js:115:27
    at compile (/Users/nw/Sites/write/node_modules/mach/modules/stack.js:88:29)
    at stack (/Users/nw/Sites/write/node_modules/mach/modules/stack.js:94:52)
    at Connection.Object.defineProperties.call (/Users/nw/Sites/write/node_modules/mach/modules/Connection.js:207:30)
    at Server.requestHandler (/Users/nw/Sites/write/node_modules/mach/modules/utils/bindApp.js:126:10)
    at Server.EventEmitter.emit (events.js:98:17)
    at HTTPParser.parser.onIncoming (http.js:2108:12)
    at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:121:23)
    at Socket.socket.ondata (http.js:1966:22)

The second problem, when I remove those lines above so it will serve my other routes, is coming from here:

app.get('*', function() { return template; });

And I get:

Error: connect ECONNREFUSED
    at errnoException (net.js:901:11)
    at Object.afterConnect [as oncomplete] (net.js:892:19)
natew commented 9 years ago

The error that's coming up actually from before the upgrade is this:

Error: Cannot find module 'qs'

I had to install qs to get it working. Am I running an old node version?

natew commented 9 years ago

Ah ok, seems like it's coming from webpack-dev-server. I'll look that way.

Edit: apologies for all the comments, still getting the ECONNREFUSED error.

mjackson commented 9 years ago

Hi @natew. The 1.0 work is backwards-incompatible with 0.12 in many ways. Most likely code that you wrote for 0.x won't work reliably on 1.0.

If you're already aware of that, you may be running into an npm issue? Sounds like you're missing some packages.

nicholascloud commented 9 years ago

I am having this exact problem. Vanilla app, no other deps except for mach. I am using 1.0.2.

'use strict';
var path = require('path');
var mach = require('mach');

var app = mach.stack();

app.use(mach.file, {
  root: path.join(__dirname, 'public'),
  index: 'index.html'
});

mach.serve(app);

Here is the output:

Error: Invalid root directory: undefined
    at file (/Users/nicholascloud/projects/temp/test-fblogin/node_modules/mach/modules/middleware/file.js:80:11)
    at Object.defineProperties.use.compiledApp (/Users/nicholascloud/projects/temp/test-fblogin/node_modules/mach/modules/middleware/stack.js:112:27)
    at compile (/Users/nicholascloud/projects/temp/test-fblogin/node_modules/mach/modules/middleware/stack.js:85:29)
    at stack (/Users/nicholascloud/projects/temp/test-fblogin/node_modules/mach/modules/middleware/stack.js:91:52)
    at Connection.Object.defineProperties.call (/Users/nicholascloud/projects/temp/test-fblogin/node_modules/mach/modules/Connection.js:210:30)
    at Server.requestHandler (/Users/nicholascloud/projects/temp/test-fblogin/node_modules/mach/modules/utils/bindApp.js:24:10)
    at Server.emit (events.js:98:17)
    at HTTPParser.parser.onIncoming (http.js:2112:12)
    at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:121:23)
    at Socket.socket.ondata (http.js:1970:22)

And a debug snapshot of the locals in the mach.file milddleware. You can see that app is null and options is undefined.

mach.file

EDIT: This also happens if I use the shorthand app.use(mach.file, path.join(__dirname, 'public')); syntax too.

nicholascloud commented 9 years ago

When the middleware is being invoked in stack, the app variable is undefined, which is causing the options variable to become undefined and the app variable to become null in the file middleware.

stack middleware

nicholascloud commented 9 years ago

It looks like this problem was introduced in 1.0.0-rc4 -- the last build that doesn't break this way is 1.0.0-rc3.

mjackson commented 9 years ago

@nicholascloud I'm looking into this. I think I know how the problem was introduced. Not sure the best way to fix it yet. Thanks for debugging :)

nicholascloud commented 9 years ago

@mjackson You are welcome, hope it helps!

mjackson commented 9 years ago

@nicholascloud @natew apologies for this bug. It was introduced in rc4 when I did some refactoring regarding the "default" app.

The app argument to middleware is now treated as opaque, which lets us pass null all the way down through the stack and define the default app in one place instead of several.

wavded commented 9 years ago

ran into this today as well, glad a fix landed!