Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add New Content-Type for Response #1

Open
taylorbrooks opened this issue Jul 16, 2014 · 5 comments
Open

Add New Content-Type for Response #1

taylorbrooks opened this issue Jul 16, 2014 · 5 comments

Comments

@taylorbrooks
Copy link

It appears that the default integration is causing unexpected issues when the response has a Content-Type of text/html.

https://next.leadconduit.com/events/53c69751cf9d919967532f70

This mime-type is currently not supported by the default integration.

https://github.com/activeprospect/leadconduit-integration-default/blob/master/src/outbound.coffee#L9

@alexkwolfe
Copy link
Contributor

When a server returns HTML, what is the desired behavior? Should it always be interpreted as a success and nothing appended?

@cgrayson
Copy link
Contributor

Maybe requestb.in is unusual, or maybe it's dumb that they give that Content-Type, but in their case it seems like it should work.

< HTTP/1.1 200 OK
< Content-Type: text/html; charset=utf-8
< Date: Thu, 17 Jul 2014 18:29:31 GMT
* Server gunicorn/18.0 is not blacklisted
< Server: gunicorn/18.0
< Sponsored-By: https://www.runscope.com
< Content-Length: 3
< Connection: keep-alive
< 
ok

Maybe when we get text/html, append what comes with it as document or body or something?

Without taking time to look one up, I don't think it's that unusual to see an HTML response in LCC deliveries. And those would have a response parsing set up to match on something like, .*Thank you for signing up!.*, say.

We want LCN response info to be automatically parsed and not need regexes, of course, but I don't know. It's a messy world out there. 😣

@alexkwolfe
Copy link
Contributor

Yeah. Thus far, the decision whether a response is a success, error, or failure is to be made by the integration's response() function. This works in the case where the integration is for a specific endpoint (say requestb.in). We know what the HTML looks like and can write a response function that interprets it accordingly.

But when the endpoint is returning some unknown HTML, we'd need a way for the user to define what constitutes each outcome. The only mechanism we have today for that is request variables, which we've defined as the data points required by the integration to formulate the request. I'm not sure how I feel about overloading request variables with regexes (or whatever) to be used during processing of the response. Maybe it's OK. Maybe not.

@cgrayson
Copy link
Contributor

Another data point, where the type is text/html but the actual body is a simple string (i.e., not real HTML at all):

untitled 3

As it happens we have a custom-built integration that they should use anyway, and they just didn't know to use that. And sure, their server is dumb for sending the wrong type header. But that's two so far, with lots more to come, I bet.

Since the last comment, we've talked about adding pattern-matching rules. If this integration set a response variable of body, then that could be evaluated with those.

Even without those, the body value could be sent on to a subsequent destination. If an endpoint returns a real full HTML page, doing that will be pretty silly. But then, people shouldn't make it do that.

If an endpoint is returning HTML that someone wants to be able to parse, like, as a structured document, then that's custom integration territory, anyway.

@alexkwolfe
Copy link
Contributor

Their server is misbehaving in this case. Our Accept header identifies 2 types of content that we can accept in the reply. Per the HTTP spec, unless they can respond with either the JSON or XML as we requested, they should return an HTTP 406. Never mind that they're returning text/plain and calling it text/html.

Maybe that's not totally relevant in so far as users don't care about the underlying details. They just want it to work. We could add a new type of integration does simple text matching on the response body to determine the outcome.

But again that's back to the whole question of whether to encourage the use of request variables in the response function.

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

No branches or pull requests

3 participants