Don't die while serializing a stack trace if __repr__ throws

[benkuhn]

As discussed in #101.

[Crisfole]

Nice! @coryvirok @brianr can we review this and get it merged? (It looks good to me).

[coryvirok]

Very nice. Can you add a test which throws an Exception in __repr__() which will break in Python 2 when we call str(e) on it?

E.g.

class CustomUnicodeRepr(object):
    def __str__(self):
        return SNOWMAN

    def __repr__(self):
        assert False

I think we will need to handle UnicodeEncodeError here https://github.com/rollbar/pyrollbar/pull/102/files#diff-f4e910d6945a21d50aae026b5f4f71b0R99

For that, I would use the text() function from https://github.com/benkuhn/pyrollbar/blob/safer_repr/rollbar/lib/__init__.py

[benkuhn]

@coryvirok Did you mean that the exception should throw a UnicodeDecodeError on __str__? This never actually calls str on the repr object, only on the exception. Here's a object that currently fails on 2.7:

class UnStringableException(Exception):
    def __str__(self):
        return SNOWMAN.decode()

class CustomRepr(object):

    def __repr__(self):
        raise UnStringableException()

Is that what you were going for? (It didn't raise with just return SNOWMAN--it needed the decode()--and I'm not up on the intricacies of py2 unicode handling...)

[coryvirok]

Ah, yes. That is what I'm looking for. Although, I don't think you need to do the .decode() since the call to str(e) will call SNOWMAN.encode() in Python 2 since it's attempting to encode the unicode into bytes and that will be the thing that throws the UnicodeEncodeError.

[coryvirok]

Oh the joys of Python 2 and Unicode. Here's an example of how we can break things:

>>> SNOWMAN = b'\xe2\x98\x83'
>>> SNOWMAN_UNICODE = SNOWMAN.decode('utf8')
>>> class Foo(Exception):
...     def __init__(self):
...         super(Foo, self).__init__(SNOWMAN_UNICODE)
...
>>> f = Foo()
>>> str(f)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2603' in position 0: ordinal not in range(128)

[benkuhn]

Hmm.

1. It seems like I really do need the decode() there--I removed it and the test didn't throw an exception.
2. I'm also not sure how you expected me to use text() since it seems to expect to take something that's already text-like, not an Exception instance. (It raised an exception when invoked directly on the Exception subclass.)

So I just wrote my own thing that catches any kind of exception in str.

[benkuhn]

Oops, looks like we crossed wires a bit! I see what the problem is--you were supplying SNOWMAN to the Exception constructor and then used the default Exception.__str__ whereas I had written my own __str__.

Anyway, I think my point 2 (and general robustness considerations) still make me lean towards the except Exception: approach I took unless you have other concerns with it.

[coryvirok]

Your implementation looks good. Is this ready to be merged and released?

[benkuhn]

As far as I know, yes!

[coryvirok]

Released v0.11.5