SiftScience / sift-python

Sift API (Python client)
MIT License
20 stars 23 forks source link

Warnings cause TypeError #33

Closed philfreo closed 8 years ago

philfreo commented 8 years ago

I'm seeing some TypeError exceptions, it seems that the new warnings / exception handling code has some issues?

  ...
  File "tasks.py", line 296, in track_sift_event
    response = sift_client.track(event_type, attrs)
  File "/home/ubuntu/closeio/venv/local/lib/python2.7/site-packages/sift/client.py", line 108, in track
    warnings.warn(traceback.format_exception_only(type(e), e))
TypeError: expected string or buffer

Not sure exactly how/when/why this happens but it seems that the Sift responses aren't always handled properly.

/cc @JohnMcSpedon @fredsadaghiani

philfreo commented 8 years ago

Any ideas?

fredsadaghiani commented 8 years ago

You're right. traceback.format_exception_only returns an array of strings, not a string. The right code would be:


except requests.exceptions.RequestException as e:
  warnings.warn('Failed to track event: %s' % properties)
  lines = traceback.format_exception_only(type(e), e)
  for line in lines:
        warnings.warn(line)

I'm not sure that the stack trace is useful in this case, so I think omitting it is best. That is, just removed the additional logging and leaving the warning about the failure to track the event.

fredsadaghiani commented 8 years ago

I'm working on a fix.

philfreo commented 8 years ago

Wouldn't having the stack trace help show where in the codebase the problem is / which usage of Sift is causing the problem?

fredsadaghiani commented 8 years ago

Yes, that's true. Usually in the case of exception, however, missing an event here or there isn't detrimental. In the degenerate case, it could spam the logs. That said, your comment is a good one and I'll make the change with the stack trace printed.

fredsadaghiani commented 8 years ago

@philfreo Please see version 1.1.2.3

philfreo commented 8 years ago

Thanks, will give it a shot!