dchester / epilogue

Create flexible REST endpoints and controllers from Sequelize models in your Express app
846 stars 116 forks source link

Probable BUG - When Restify server has a different name than 'restify', epilogue will crash #144

Closed ilaif closed 8 years ago

ilaif commented 8 years ago

Hey guys,

Looks like a great package! Should save us a lot of work.

I've ran into a problem: When trying to initialize epilogue with a Restify server with a different name than default, epilogue crashes with the following error:

/usr/local/bin/node --debug-brk=56479 --nolazy api.js Debugger listening on port 56479 /Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Controllers/base.js:76 app[self.method](endpoint.string, function(req, res) { ^ TypeError: app[self.method] is not a function at Controller.route (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Controllers/base.js:76:19) at Controller.initialize (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Controllers/base.js:33:8) at Controller (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Controllers/base.js:9:8) at new Delete (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Controllers/delete.js:8:17) at null. (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Resource.js:69:32) at Array.forEach (native) at new Resource (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/Resource.js:65:16) at Object.epilogue.resource (/Users/ilaif/git/analoc/bolt/node_modules/epilogue/lib/index.js:56:20) at Object.db.init.Object.keys.forEach as init at Object. (/Users/ilaif/git/analoc/bolt/src/api/server.js:57:8) at Module._compile (module.js:399:26) at Object.Module._extensions..js (module.js:406:10) at Module.load (module.js:345:32) at Function.Module._load (module.js:302:12) at Module.require (module.js:355:17) at require (internal/module.js:13:17) Process finished with exit code 1

I've traced down the error and found the following code:

// NOTE: is there a better place to do this mapping?
  if (app.name === 'restify' && self.method === 'delete')
    self.method = 'del';

  app[self.method](endpoint.string, function(req, res) {
    self._control(req, res);
  });

Notice that "app.name === 'restify' is not a good identifier for an app being restify since restify's api allows to change the restify server name when creating the server (mostly for debugging purposes) like so:

var server = restify.createServer({
    name: 'some-other-name-than-restify',
});

I've named my restify server as 'restify' and it solved the problem.

Requesting a fix. I'm not familiar enough to understand how restify vs express affects the internals of the code.

Thanks a lot in advance, Ilai

mbroadst commented 8 years ago

@ilaif hey looks like a duplicate of #133 perhaps? I think @cusspvz had a good idea of polyfilling, but no work as yet been submitted to address the issue. Would you perhaps be interested in taking a stab at it? (I've found myself more in a mediator role than maintainer these days :smile:)

ilaif commented 8 years ago

Yes it is indeed an exact duplicate. I will start working with epilogue and when I understand enough I will submit a pr.

Thanks for the quick response! Ilai

cusspvz commented 8 years ago

I'm hoping to place here up to 3 contributions, since I'm gonna need them on Friday. This could be one of them. ;)

mbroadst commented 8 years ago

closing in favor of #133