reactioncommerce / reaction-cli

A command line tool for working with Reaction Commerce.
33 stars 20 forks source link

Add exception for inspect-brk and inspect arguments #79

Closed loan-laux closed 5 years ago

loan-laux commented 5 years ago

This is a fix for issue #78. It makes sure that --inspect-brk works as expected and doesn't make the app crash on startup.

focusaurus commented 5 years ago

I can't get this to work yet for the full matrix of --inspect, --inspect-brk, --inspect=0.0.0.0:9229, --inspect-brk=0.0.0.0:9229. Might need a little tweaking to handle both flags with and without arguments. The 0.0.0.0:9229 flavor is needed when running reaction inside a docker container.

focusaurus commented 5 years ago

Looks like maybe we end up passing both kebab case and camel case versions to meteor:

{ Error: Command failed: meteor --inspect-brk --inspectBrk --raw-logs
    at checkExecSyncError (child_process.js:601:13)
    at execSync (child_process.js:641:13)
    at _callee$ (/usr/local/lib/node_modules/reaction-cli/dist/commands/run.js:72:43)
    at tryCatch (/usr/local/lib/node_modules/reaction-cli/node_modules/regenerator-runtime/runtime.js:65:40)
    at Generator.invoke [as _invoke] (/usr/local/lib/node_modules/reaction-cli/node_modules/regenerator-runtime/runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/reaction-cli/node_modules/regenerator-runtime/runtime.js:117:21)
    at step (/usr/local/lib/node_modules/reaction-cli/dist/commands/run.js:98:191)
    at /usr/local/lib/node_modules/reaction-cli/dist/commands/run.js:98:361
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
  error: null,
  cmd: 'meteor --inspect-brk --inspectBrk --raw-logs',
  file: '/bin/sh',
  args: 
   [ '/bin/sh',
     '-c',
     'meteor --inspect-brk --inspectBrk --raw-logs' ],
loan-laux commented 5 years ago

@focusaurus Hope you've had a good Christmas. Sorry about --inspect, I had forgotten about it since my own pain-point was with --inspect-brk. Both should work with and without values now.

On your last comment, I don't recall any case where both camel case and kebab case arguments/flags would be passed to Meteor. However, this seemed to be the default behavior before my PR if I remember well. I'd say try and pull the branch with my new changes, see if that helps.

focusaurus commented 5 years ago

OK I tested your most recent commit and all 4 flavors are working. Thanks for this PR!