cloudfoundry-community / node-cfenv

easy access to your Cloud Foundry application environment for node
Apache License 2.0
74 stars 20 forks source link

improve error message on bad manifest.yml #25

Closed srl295 closed 4 years ago

srl295 commented 7 years ago

Less than helpful stack trace on erroneous manifest.yml

Caused this stack trace when running the app: (note: it "used to work", perhaps some dependency changed?)

$ node -p 'require("cfenv").getAppEnv()'

/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:168
  throw generateError(state, message);
  ^
Error
    at generateError (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:162:10)
    at throwError (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:168:9)
    at storeMappingPair (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:305:7)
    at readBlockMapping (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1066:9)
    at composeNode (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1327:12)
    at readBlockSequence (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:925:5)
    at composeNode (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1358:45)
    at readBlockMapping (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1057:11)
    at composeNode (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1327:12)
    at readDocument (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1489:3)

but:

$ node -p 'try{ require("cfenv").getAppEnv(); } catch(e) { console.error(e); }'
{ Error
    at generateError (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:162:10)
    at throwError (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:168:9)
    at storeMappingPair (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:305:7)
    at readBlockMapping (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1066:9)
    at composeNode (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1327:12)
    at readBlockSequence (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:925:5)
    at composeNode (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1358:45)
    at readBlockMapping (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1057:11)
    at composeNode (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1327:12)
    at readDocument (/Users/srl/src/gp-nodejs-sample/node_modules/cfenv/node_modules/js-yaml/lib/js-yaml/loader.js:1489:3)
  name: 'YAMLException',
  reason: 'duplicated mapping key',
  mark: 
   Mark {
     name: 'manifest.yml',
     buffer: '---\napplications:\n- name: gp-nodejs-sample\n  services:\n    - Globalization Pipeline-ec\n  disk_quota: 384M\n  host: gp-nodejs-sample\n  name: gp-nodejs-sample\n  path: .\n  domain: mybluemix.net\n  instances: 1\n  memory: 128M\n\u0000',
     position: 155,
     line: 7,
     column: 24 },
  message: 'duplicated mapping key in "manifest.yml" at line 8, column 25:\n      name: gp-nodejs-sample\n                            ^' }
undefined

I'm not sure why the message is just Error in the first place. Perhaps catching this error from YAML and adding some information would help? Otherwise code such as the quickstart throws an unhelpful stack trace.

srl295 commented 7 years ago

Showed up in https://github.com/IBM-Bluemix/gp-nodejs-sample/issues/11

pmuellr commented 7 years ago

(note: it "used to work", perhaps some dependency changed?)

What does it "used to work" mean? Ignored the duplicate key, and ran without any warnings?

Perhaps catching this error from YAML and adding some information would help?

I'm seeing a more elaborate message:

$ npm install cfenv --save
test-cfenv@1.0.0 /Users/pmuellr/tmp/test-cfenv
└─┬ cfenv@1.0.4
  ├─┬ js-yaml@3.7.0
  │ ├─┬ argparse@1.0.9
  │ │ └── sprintf-js@1.0.3
  │ └── esprima@2.7.3
  ├── ports@1.1.0
  └── underscore@1.8.3

$ cat manifest.yml
applications:
- name:      cf-env-test
  name:      duplicate-name

$ node -p 'require("cfenv").getAppEnv()'

/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:168
  throw generateError(state, message);
  ^
YAMLException: duplicated mapping key in "manifest.yml" at line 3, column 28:
      name:      duplicate-name
                               ^
    at generateError (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:162:10)
    at throwError (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:168:9)
    at storeMappingPair (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:305:7)
    at readBlockMapping (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:1066:9)
    at composeNode (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:1327:12)
    at readBlockSequence (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:925:5)
    at composeNode (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:1358:45)
    at readBlockMapping (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:1057:11)
    at composeNode (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:1327:12)
    at readDocument (/path/to/node_modules/js-yaml/lib/js-yaml/loader.js:1489:3)

I guess the question is - why aren't you seeing this?

That seems like the appropriate level of detail - assuming we want to consider this to be an error-throwing situation.

This is really just for non-CF usage, right? Seems like it only reads the manifest.yml if no name is in VCAP_APPLICATION, which shouldn't happen I think.

If so, I think throwing the exception is appropriate - if it was something that could happen in CF, I'd be more likely to consider ignoring it.

pmuellr commented 7 years ago

Any other info @srl295 ? Still curious why your node -p 'require("cfenv").getAppEnv()' didn't print the yaml error message but mine did.

I checked out the bluemix project, and tried to reproduce the same error-message-free npm run start, and ... it also printed the yaml error message, unlike what was logged in the bluemix bug.

What version of node are you running? Could you do a npm ls and provide the output here? Here's what I get for the cfenv portion, on my box:

├─┬ cfenv@1.0.4
│ ├─┬ js-yaml@3.7.0
│ │ ├─┬ argparse@1.0.9
│ │ │ └── sprintf-js@1.0.3
│ │ └── esprima@2.7.3
│ ├── ports@1.1.0
│ └── underscore@1.8.3
pmuellr commented 7 years ago

@srl295 srl295 - ping!

I'll close this in a few days if there's no response.

pmuellr commented 4 years ago

closing as it's inactive