Skip to content
This repository has been archived by the owner on Sep 25, 2020. It is now read-only.

tcurl crashes when receiving non-ok result #32

Open
benfleis opened this issue Aug 11, 2015 · 6 comments
Open

tcurl crashes when receiving non-ok result #32

benfleis opened this issue Aug 11, 2015 · 6 comments

Comments

@benfleis
Copy link

[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
Copy link
Contributor

Raynos commented Aug 11, 2015

@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
Copy link

jwolski commented Aug 12, 2015

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

@benfleis
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Contributor

Raynos commented Aug 12, 2015

@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
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants