apigee-127 / swagger-node-runner

The heart of Swagger-Node
MIT License
102 stars 123 forks source link

Config module prolem #39

Closed hotab closed 7 years ago

hotab commented 8 years ago

I tried to use swagger-node (with express) with the config module. I was loading all the config first and then used SwaggerExpress.create. This module was throwing "Cannot assign to read only property 'swagger' of [object Object]", stack trace:

TypeError: Cannot assign to read only property 'swagger' of [object Object] at new Runner ([cut]\node_modules\swagger-node-runner\index.js:154:18) at Object.module.exports.create ([cut]\node_modules\swagger-node-runner\index.js:54:3) at Object.module.exports.create ([cut]\node_modules\swagger-express-mw\lib\index.js:30:10) at Object. ([cut]\app.js:12:16) at Module._compile (module.js:413:34) at Object.Module._extensions..js (module.js:422:10) at Module.load (module.js:357:32) at Function.Module._load (module.js:314:12) at Function.Module.runMain (module.js:447:10) at startup (node.js:141:18) at node.js:933:3

I debugged the problem and used a (kinda) dirty hack - I load SwaggerExpress (and hence swagger-node-runner) before anything else, and suddenly it works. Anyway, it was not an expected behavior

I researched it deeper, and understood that this is related to the config object becoming immutable

So you should either update the docs with a note that this module should be loaded prior to config module reads (it writes to swagger variable), or alter the inner structure somehow.

theganyo commented 8 years ago

Thanks for reporting this. You're correct, this should be added to the documentation.

Jarvie8176 commented 8 years ago

an alternative solution is to use environment variable ALLOW_CONFIG_MUTATIONS, per https://github.com/lorenwest/node-config/wiki/Environment-Variables#allow_config_mutations

hotab commented 8 years ago

As per description: "If this environment variable contains anything, then mutations of the configuration object are allowed. This is for edge cases such as testing, where it is important to mutate configurations for different scenarios within the same execution.

The safety afforded by making the configuration object immutable is lost when this environment variable is used."

It seems that the authors of config module had some considerations against mutability, so it might not be a good idea to rely on this option.

Jarvie8176 commented 8 years ago

How about enabling it for swagger to load, then change the variable back to it's previous value?

To my understanding node-config checks against that variable whenever there is a mutation attempt.

zuumapp commented 8 years ago

Any update on this? What is weird is that locally my swagger project starts fine but on prod (ec2 aws) I receive the same error:

Cannot assign to read only property 'swagger' of object '[object Object]'

Hacky solution doesn't work for me. I load swagger express first but still the same error is thrown.

var SwaggerExpress = require('swagger-express-mw');
var SwaggerUi = require('swagger-tools/middleware/swagger-ui');
var config = require('config');
var mongoose = require('./config/mongoose');
var morgan = require('morgan');
var express = require('./config/express');
love8587 commented 8 years ago

@zuumapp I have struggled with this for a few hours. I want it will be helpful to you.

'use strict';

var nodeConfig = require('config');
var logger = require('morgan');
var SwaggerExpress = require('swagger-express-mw');
var app = require('express')();

var config = {
  appRoot: __dirname // required config
};

SwaggerExpress.create(config, function(err, swaggerExpress) {
  if (err) { throw err; }

  // install middleware
  swaggerExpress.register(app);

  var port = process.env.PORT || 10010;
  app.listen(port);

  if (swaggerExpress.runner.swagger.paths['/hello']) {
    console.log('try this:\ncurl http://127.0.0.1:' + port + '/hello?name=Scott');
  }
});

app.use(logger(nodeConfig.get('server.logger.format')));

module.exports = app; // for testing
cristiboariu commented 7 years ago

I also encounter this bug. Is there any way to solve it?

It only happens on EC2 machine but it works locally. I have added:

process.env['ALLOW_CONFIG_MUTATIONS'] = true

but still the same issue happens:

TypeError: Cannot assign to read only property 'swagger' of object '[object Object]' at new Runner (/var/lib/jenkins/workspace/project/node_modules/swagger-node-runner/index.js:154:18) at Object.module.exports.create (/var/lib/jenkins/workspace/project/node_modules/swagger-node-runner/index.js:54:3) at Object.module.exports.create (/var/lib/jenkins/workspace/project/node_modules/swagger-express-mw/lib/index.js:30:10)

@love8587 can you please let me know where exactly the fix is in your answer?

cristiboariu commented 7 years ago

Updated "swagger-express-mw": "^0.1.0" to "swagger-express-mw": "^0.6.0" and it works now.

Louis-at-Rappler commented 7 years ago

process.env['ALLOW_CONFIG_MUTATIONS'] = true do the trick on my end

diegoalex commented 6 years ago

I've updated the "swagger-express-mw": "^0.1.0" to "swagger-express-mw": "^0.6.0" as @cristiboariu suggested and I started to get this error on all api calls =(

if I set the version "^0.1.0" it works fine (but not the config)

TypeError: Cannot read property 'code' of undefined at sendSecurityError (/project-path/node_modules/swagger-node-runner/fittings/swagger_security.js:93:11) at orCheckDone (/project-path/node_modules/swagger-node-runner/fittings/swagger_security.js:69:26) at /project-path/node_modules/swagger-node-runner/node_modules/async/lib/async.js:52:16 at /project-path/node_modules/swagger-node-runner/node_modules/async/lib/async.js:361:13 at /project-path/node_modules/swagger-node-runner/node_modules/async/lib/async.js:52:16 at async.forEachOf.async.eachOf (/project-path/node_modules/swagger-node-runner/node_modules/async/lib/async.js:236:30) at _asyncMap (/project-path/node_modules/swagger-node-runner/node_modules/async/lib/async.js:355:9) at Object.map (/project-path/node_modules/swagger-node-runner/node_modules/async/lib/async.js:337:20) at swagger_security (/project-path/node_modules/swagger-node-runner/fittings/swagger_security.js:34:11) at Runner. (/project-path/node_modules/bagpipes/lib/bagpipes.js:171:7) at bound (domain.js:301:14) at Runner.runBound (domain.js:314:12) at Runner. (/project-path/node_modules/pipeworks/pipeworks.js:72:17) at postFlight (/project-path/node_modules/bagpipes/lib/bagpipes.js:220:3) at /project-path/node_modules/bagpipes/lib/bagpipes.js:174:9 at cors (/project-path/node_modules/cors/lib/index.js:188:7)

theganyo commented 6 years ago

@diegoalex You must make some changes when upgrading. Please read the release notes (see the readme).

avtaniket commented 6 years ago

Adding _ALLOW_CONFIGMUTATIONS fixed issue for me

process.env['ALLOW_CONFIG_MUTATIONS'] = true;
const appConfig = require('config');