segmentio / integration Goto Github PK

i think having .enabled() on the proto is nice, but integrations shouldn't override it.

I'm thinking we could add something similar to how analytics.js has stuff like 'asssumesPageview()'. Basically a few flags you can set to indicate where the integration is enabled. Maybe server() is called automatically:

var Webhooks = module.exports = integration('Webhooks')
  .retries(2)
  .mobile()
  .client();

This would be added to the integration tester as well, and then we could do away with all of those .enabled() checks in our tests. We could just check that .server() once enables the server channel and then have integration tester assert that the integration uses .server()

Realizing while talking with @sperand-io that we'll probably want to come up with a way to send page calls for services which don't offer first-class support for it. Right now we have a few different settings for this kind of thing, like trackAllPages, trackCategorizedPages, trackNamedPages, and they tend to get out of sync between server and client.

Actually now that I'm thinking about it, maybe we should just add it to the proto for any integrations which don't explicitly have a .page call. Thoughts?

@yields @gjohnson @ianstormtaylor

We can requeue instead internally.

Wanted to propose a .retry method. Basically this would have a default implementation but allows for each integration to implement it differently if needed.

For example, an integration that should only be retried upon a ratelimit response:

Example.prototype.retry = function(er){
 return err.status == 429;
};

The default would probably be:

Integration.prototype.retry = function(err){
  return !!~[502,503,504].indexOf(err.status)
    || err.code == 'ETIMEDOUT'
    || err.code == 'EADDRINFO'
    || err.code == 'ECONNRESET'
    || err.code == 'ESOCKETTIMEDOUT'

Internally we would leverage this in our current requeue logic once we remove superagent-retry.

CC: @yields @calvinfo

basically make it really easy to assert request and response, inspired by supertest.

so we can add tests to data sent and received.

tester(integration)
  .identify(identify)
  .expect('content-type', 'application/json')
  .body({ created_at: Date })
  .body({ apiKey: '8cd9aacf' })
  .body({ user_id: '2cc8025d' })
  .response({ status: 100 })
  .end(done)

So many of these guys require odd date formats for their values. Might be worth adding methods to the prototype indicating that we should convert any object values which match an iso8601 string to unix timestamps

CustomerIO = integration('Customer.io')
  .mapper(mapper) // unix timestamps get passed in here
  .retries(2)
  .unixtime()

cc @yields

Now that I'm thinking about it, it might be useful to have a flag for full debug req/response, something similar to what curl -v gives you. What do you think @yields @gjohnson ?

It will be easier to toggle output, plus debug is just another level on a logger. No need to have both right?

args: type, msg, ctx.

if (o) return fn(this.error('missing_setting', 'setting "foo" is missing', settings));

for context we can add the integration name.

but not sure about it still, i prefer assert() or this.assert() and make it throw and let the caller will catch it, it will be a lot cleaner.

on the other hand it's weird since you supply a callback and you expect fn(err).

/cc @calvinfo

we probably want all requests to have a timeout of 10s or so and then let this be adjustable too, just in case any integrations start doing weird things.

I notice a bunch of the providers require an email, might be nice to have an

Integration
  .requires('email')
  .requires('userId')

or even an

Integration
  .email()

so that we can start taking out the custom .enabled calls

to avoid regressions

so we can debug in prod, similar to all the debug calls currently