search

uber-node / tcurl

A command line utility to talk to tchannel servers
MIT License
36 stars 7 forks source link

tcurl crashes when receiving non-ok result #32

Open benfleis opened 9 years ago

benfleis commented 9 years ago 
[benfleis] ~/src/ringpop $ for i in 0; tcurl -p 10.80.134.147:300$i ringpop /trace/remove -2 '{ "event": "membership.update", "sink": {"type": "log"}}'

/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/as/json.js:299
            self.logger.warn('Got unexpected invalid JSON for arg2', {
                        ^
TypeError: Cannot call method 'warn' of undefined
    at TChannelJSON.parse [as _parse] (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/as/json.js:299:25)
    at onResponse (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/as/json.js:125:32)
    at gotArg23 (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/request.js:134:13)
    at TChannelInResponse.withArg23 (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/in_response.js:88:5)
    at DefinedEvent.onResponse [as listener] (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/request.js:133:13)
    at DefinedEvent.emit (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/lib/event_emitter.js:86:14)
    at TChannelRequest.emitResponse (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/request.js:106:24)
    at TChannelRequest.onSubreqResponse (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/request.js:264:14)
    at DefinedEvent.onResponse [as listener] (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/request.js:242:14)
    at DefinedEvent.emit (/Users/benfleis/.nvm/v0.10.26/lib/node_modules/tcurl/node_modules/tchannel/lib/event_emitter.js:86:14)

My client was temporarily hard coded to resp.sendNotOk().

When looking at said as/json.js code, I don't see why it would be null. I will try a little more debugging, but perhaps somebody over there has more insight into this. (I am also unsure whether it's fundamentally a tchannel problem, or a tcurl problem.)

Raynos commented 9 years ago

@benfleis an empty string is invalid json.

The json arg scheme for tchannel requires json in arg2 and arg3.

You should send '{}' or 'null'

jwolski commented 9 years ago

Yes, but tchannel can give a lot better of an error.

benfleis commented 9 years ago

Agreed on error handling for such a situation. Perhaps I misunderstood -- is the tchannel/tcurl contract such that all data must be JSON? i.e., can you never send an Ok with no data?

kriskowal commented 9 years ago

@benfleis For the interim, TCurl accepts JSON for arg2 and arg3 for JSON and Thrift argschemes, or raw strings for the raw argscheme. It could be taught to use {} Thrift by default (or null if we adjust Thriftifty/ThriftRW to accept null in place of an object with all optional properties). I also have this crazy idea of finishing Argunauts to make it a little more natural to express JSON as command line arguments.

Raynos commented 9 years ago

@jwolski agreed. We need to pass a logger into tcurl.

@benfleis if you want to do raw encoding then we have a --raw flag.

If you want an easier way to do json then you can import tchannel/as/json and it will do the correct thing for you.

kriskowal commented 9 years ago

I concur with @jwolski. This issue remains open until we provide a better error.