MediaMath / t1-python

Python SDK for MediaMath Platform APIs
https://developer.mediamath.com/
Apache License 2.0
18 stars 30 forks source link

ClientError: 'Cannot handle content type: text/html' on t1.get('strategies', 2021657, child='region') #137

Closed robrechtdr closed 7 years ago

robrechtdr commented 7 years ago

Ran with version with latest commit https://github.com/robrechtdr/t1-python/commit/294e3be7a8477201cf3ef041e1229e80b04d9c13 (is cloned version, as previously reported other error occurs otherwise)

> t1.get('strategies', 2021657, child='region')

Traceback (most recent call last):
  File "/Users/rrouck/.virtualenvs/pi-automation-api-dev/lib/python2.7/site-packages/werkzeug/wsgi.py", line 703, in __next__
    return self._next()
  File "/Users/rrouck/.virtualenvs/pi-automation-api-dev/lib/python2.7/site-packages/werkzeug/wrappers.py", line 81, in _iter_encoded
    for item in iterable:
  File "/Users/rrouck/pi-automation-api/automation_api/runners.py", line 789, in create_targets
    strategy_targeting = t1.get('strategies', strategy_id, child=t1_dimension)
  File "/Users/rrouck/.virtualenvs/pi-automation-api-dev/lib/python2.7/site-packages/terminalone/service.py", line 363, in get
    entities, ent_count = super(T1, self)._get(self._get_service_path(collection), _url, params=_params)
  File "/Users/rrouck/.virtualenvs/pi-automation-api-dev/lib/python2.7/site-packages/terminalone/connection.py", line 187, in _get
    return self._parse_response(response)
  File "/Users/rrouck/.virtualenvs/pi-automation-api-dev/lib/python2.7/site-packages/terminalone/connection.py", line 213, in _parse_response
    raise ClientError('Cannot handle content type: {}'.format(content_type))
ClientError: 'Cannot handle content type: text/html'
FodT commented 7 years ago

@robrechtdr is this still happening? a content type header of text/html should never be returned from adama, I'm wondering if there was a nginx snafu.

robrechtdr commented 7 years ago

Looks like it, the endpoint we were running that got stuck on this issue now processed through.

robrechtdr commented 7 years ago

Problem is still occurring frequently on consecutive runs (something we need to do) with diff params. Can anyone working on Adama check this?

The corresponding get request uses the url of the following form: https://t1qa10.mediamath.com/api/v2.0/strategies/2021487/target_dimensions/7.

It seems to appear intermittently, seemingly independent from particular entries. E.g. for the above case, first time I called it got the text/html returned but not the second time.

FodT commented 7 years ago

Ah, this is on qa10? I would ask in the current issues: qa room.

robrechtdr commented 7 years ago

Seems to also occur on production.

robrechtdr commented 7 years ago

Failing case:

vs

Passing case:

FYI @FodT

furiouslyalex commented 7 years ago

This error is still happening on 2 apps we've updated to 1.6.1 for deals/media migration.

Bulk_api and pmp_contextual_assigner.

Current solution is to add a try/except around the content type check in connection.py in a local version of t1-python.

@FodT thinks this simple solution might work: "well i can probably have some code to handle an empty response and return a T1Error"

Thanks, Alex

conoverm commented 7 years ago

I think it's most likely the case this error occurs when the API returns some sort of non-formatted error response like an NGINX HTML error page. I've seen that often. Hopefully, all it needs an error catch on responses.

FodT commented 7 years ago

@conoverm @furiouslyalex Looking at the response handling, I'm not sure what would be more 'valid' behaviour outside of throwing a T1Error instead of a ClientError (which should only be thrown on actual improper SDK usage).

The fact nginx is returning 400 responses on actual valid responses is a separate issue which we shouldn't be trying to work around on the SDK.

In either case, client code has to retry the request, so, I'm asking, what do you want to see t1-python doing here?

conoverm commented 7 years ago

Understood. It does make the issue easier to diagnose. What do you think Alex?

FodT commented 7 years ago

Agreed, I missed the case where the content type is missing.

furiouslyalex commented 7 years ago

fyi @FodT I tested the branch you PR'd from your fork and it fixes my content type problem. Thanks.