Untangle

[kriskowal]

These are a few initial steps toward untangling the control flow, complexity, and bad UX that has accumulated in TCurl.

• Any text that isn’t a parse-able response gets shunted to stderr.
• Dumps JSON for as=json and as=thrift responses. There is no longer a util.inspect mode. Just use jq to pick it apart.
• Dumps the entire response object to the console, including "ok", "head", "body", and "headers". Use jq to pick out what you need.
• No longer logs "Got call response ok" or "Got call response not ok". Just dumps the response, which includes an "ok" property anyway, and the exit code reflects that. (In the future, we might lift the code right from the TChannel frame, but that is not possible with the As* layer from TChannel today). Application errors are dumped to STDOUT so they can be picked at with jq. Protocol errors are written to stderr and set the exit code to 255.
• No longer creates a {message, response} envelope when dumping in JSON format.
• Consolidates a bunch of logic in a delegate “logger” object, that has error(), response(), and exit() methods that TCurl calls into. error() may be called any number of times. exit() calls process.exit() with the appropriate status code when run as main. Each test defines its own delegate.
• Health check code only differs substantially by small changes to the logger. Instead of dumping JSON, dumps the same text as previous. This eliminates the redundancies in the former meta-client.js, just defaulting a couple options and using a slightly different logger.
• I turned TCurl into a prototype. This turned out to have little benefit but I did it anyway. Turns out TCurl has no state. All the state gets threaded through tcurl.request via opts and delegate.
• The factoring for asHTTP and asThrift is still obtuse, but one step at a time. There is also no way to get raw binary thrift output because of coupling in TChannelAsThrift. May consider using ThriftRW directly to decouple how input and output args are encoded and decoded.

[kriskowal]

r @ShanniLi @Raynos @jcorbin

Please give this a spin to assess the usability. Example:

before❯ node index.js -p 127.0.0.1:21300 -t ~/larry/thrift/stooges.thrift larry Stooges::nyuck
'Got call response ok'
{ name: 'larry', language: 'python' }

after❯ node index.js -p 127.0.0.1:21300 -t ~/larry/thrift/stooges.thrift larry Stooges::nyuck | jq
{
  "ok": true,
  "head": {},
  "body": {
    "name": "larry",
    "language": "python"
  },
  "headers": {
    "as": "thrift"
  }
}

❯ node index.js -p 127.0.0.1:21300 autobahn connections_v1 -3 '{"serviceName": "larry"}' | jq
{
  "ok": true,
  "head": null,
  "body": {
    "127.0.0.1:21300": {
      "instances": {
        "10.32.193.49:56903": {
          "connected": {
            "in": true,
            "out": true
          },
          "identified": {
            "in": true,
            "out": true
          },
          "serviceNames": [
            "larry"
          ]
        }
      }
    },
    "127.0.0.1:21301": {
      "instances": {
        "10.32.193.49:56903": {
          "connected": {
            "in": true,
            "out": true
          },
          "identified": {
            "in": true,
            "out": true
          },
          "serviceNames": [
            "larry"
          ]
        }
      }
    }
  },
  "headers": {
    "as": "json"
  }
}

[Raynos]

@kriskowal

Can you show an example of "error frame", "not ok response" and "ECONNREFUSED" with this new tcurl ?

[kriskowal]

@Raynos, stderr as follows, exit code 1.

{ [TchannelBadRequestError: Endpoint 'Meta::health' is not defined]
  type: 'tchannel.bad-request',
  message: 'Endpoint \'Meta::health\' is not defined',
  isErrorFrame: true,
  codeName: 'BadRequest',
  errorCode: 6,
  originalId: 2,
  name: 'TchannelBadRequestError',
  fullType: 'tchannel.bad-request' }

I believe from our offline discussion you would like this to be stdout. I think it would be good to surface the "errorCode" as the exit code, so 6.

{
    "isErrorFrame": true,
    "message": "Endpoint \'Meta::health\' is not defined",
    "codeName": "BadRequest",
    "errorCode": 6,
    "originalId": 2,
    "name": "TchannelBadRequestError",
    "fullType": "tchannel.bad-request"
}

[kriskowal]

Feedback fed back. Please yay or nay.

[jcorbin]

:8ball: says :ship:

[ShanniLi]

Would appreciate answering the question about assert.ifError v.s. if (err) { assert.end(err) }

Other than that, lgtm.

[kriskowal]

Yeah, thought I had. assert.ifError is non-fatal and does not stop the test, just as delegate.error is non-fatal and does not indicate that tcurl is finished. That is the sole purview of delegate.exit. Thanks for your feedback!

[ShanniLi]

Make sense. Thanks! @kriskowal