kennethreitz / crayons

Text UI colors for Python.
https://pypi.python.org/pypi/crayons
MIT License
423 stars 30 forks source link

If an `AttributeError` is raised in `ColoredString.color_str`, python will raise a `TypeError` when printing #26

Closed maartendp closed 4 years ago

maartendp commented 4 years ago

Hi,

The color_str property is checking if the stdout is a tty by running sys.stdout.isatty().

In our case, a 3rd party populated the sys.stdout with an object that did not implement isatty. Because of this, an AttributeError was thrown when running the code in color_str. Python implicitly caught this and defaulted to getting the value from __getattr__, which is, as you know, a function and indeed not a compatible type for printing.

An implicit fix would be

@property
def color_str(self):
    try:
        return self._color_str()
    except Exception:
        return self.s

def _color_str(self):
    # current existing code

An explicit fix would be

@property
def color_str(self):
    try:
        return self._color_str()
    except AttributeError as e:
        raise MyCustomError("An error was thrown when assembling the colored string: {}".format(e))

def _color_str(self):
    # current existing code

But I will leave it to your judgement to find the best approach.

Thanks for your time.

MasterOdin commented 4 years ago

I think we could also just modify the check elif sys.stdout.isatty() and not DISABLE_COLOR: to be elif hasattr(sys.stdout, 'isatty') and sys.stdout.isatty() and not DISABLE_COLOR:

In any case, this should definitely be an implicit check, not an explicit check, as I would not classify colored output in abnormal environments mission critical.

maarten-dp commented 4 years ago

Even better :) thanks for the quick reply.

MasterOdin commented 4 years ago

This is fixed in the new 0.4.0 release

maarten-dp commented 4 years ago

Thank you for taking care of this so quickly. Very much appreciated!