manrajgrover / halo

πŸ’« Beautiful spinners for terminal, IPython and Jupyter
MIT License
2.89k stars 147 forks source link

Bug: exceptions are swallowed by __exit__ #102

Closed theY4Kman closed 5 years ago

theY4Kman commented 5 years ago

Description

By returning a truthy value from __exit__, exceptions thrown when using Halo as a context manager are swallowed, and execution continues.

System settings

Expected behaviour

βœ– Doing the thing ...
Traceback (most recent call last):
  File "example.py", line 1337, in example
    raise Exception('stuff')
Exception: stuff

Steps to recreate

from halo import Halo

with Halo('Doing the thing ... '):
    # No exception seen
    raise Exception('stuff')
from halo import Halo

class TransparentHalo(Halo):
    def __exit__(self, exc_type, exc_value, traceback):
        super().__exit__(exc_type, exc_value, traceback)
        # no return

with TransparentHalo('Doing the thing ... '):
    raise Exception('stuff')
manrajgrover commented 5 years ago

@theY4Kman Thanks for raising this issue! It makes sense to raise an exception after stopping the spinner. Would you like to raise a PR for this?

theY4Kman commented 5 years ago

Haha! Yeah, might be tonight or tomorrow.

theY4Kman commented 5 years ago

I tried running tox yesterday and had some issues. I tried again tonight and found the same (go figure :P though, I was missing py34 yesterday, so I tried again with it installed, just to be thorough): https://gist.github.com/theY4Kman/c9dbeffe44fce618f3fa402079073568

Though the main change in the PR would prolly be one-line β€” even if I really want to add an __init__ kwarg to allow exception swallowing β€” I ain't gonna submit shit without tests. And to be totally honest, I ran those tests and am writing this in between development I've been told not to do so much of at this hour... So I might not get to figuring out my own stupid environment ('cause the travis tests pass just fine) for a while.

In any case, man, thank you for some sexy-ass spinners. tqdm is really fuckin complicated, and after I figured out enough to appease me... I learned it didn't support indefinite progress at all :P

Cheers!

manrajgrover commented 5 years ago

@theY4Kman Strange! The test cases should pass just fine locally. Since only animation tests are failing, I think this might be because of terminal size issues. Maybe you can check if it is because of the same. Please raise a PR/share the code and let me also evaluate.

You're welcome! I'm trying my best to make this project complete and easy to develop with.

manrajgrover commented 5 years ago

@theY4Kman I just merged #99 which should fix this issue for you.

theY4Kman commented 5 years ago

Aww yiss! That did it! Thank you :) The linter fails with this guy I haven't had time to diagnose, yet, but with an interesting traceback like that, I'm dying to know what the hell it's doing

lint recreate: /Users/they4kman/programming/third-party/halo/.tox/lint
lint installdeps: -r/Users/they4kman/programming/third-party/halo/requirements.txt, -r/Users/they4kman/programming/third-party/halo/requirements-dev.txt
lint inst: /Users/they4kman/programming/third-party/halo/.tox/dist/halo-0.0.18.zip
lint installed: appnope==0.1.0,astroid==2.0.4,bleach==3.0.2,colorama==0.3.9,coverage==4.4.1,cursor==1.2.0,decorator==4.3.0,defusedxml==0.5.0,entrypoints==0.2.3,enum34==1.1.6,halo==0.0.18,ipykernel==5.1.0,ipython==5.7.0,ipython-genutils==0.2.0,ipywidgets==7.1.0,isort==4.3.4,Jinja2==2.10,jsonschema==2.6.0,jupyter-client==5.2.3,jupyter-core==4.4.0,lazy-object-proxy==1.3.1,log-symbols==0.0.11,MarkupSafe==1.0,mccabe==0.6.1,mistune==0.8.4,nbconvert==5.4.0,nbformat==4.4.0,nose==1.3.7,notebook==5.7.0,pandocfilters==1.4.2,pexpect==4.6.0,pickleshare==0.7.5,pluggy==0.8.0,prometheus-client==0.4.2,prompt-toolkit==1.0.15,ptyprocess==0.6.0,py==1.7.0,Pygments==2.2.0,pylint==1.7.2,python-dateutil==2.7.3,pyzmq==17.1.2,Send2Trash==1.5.0,simplegeneric==0.8.1,six==1.11.0,spinners==0.0.23,termcolor==1.1.0,terminado==0.8.1,testpath==0.4.2,tornado==5.1.1,tox==2.8.2,traitlets==4.3.2,virtualenv==16.0.0,wcwidth==0.1.7,webencodings==0.5.1,widgetsnbextension==3.1.4,wrapt==1.10.11
lint runtests: PYTHONHASHSEED='3195813463'
lint runtests: commands[0] | pylint --errors-only --rcfile=/Users/they4kman/programming/third-party/halo/.pylintrc --output-format=colorized halo
Traceback (most recent call last):
  File "/Users/they4kman/programming/third-party/halo/.tox/lint/bin/pylint", line 11, in <module>
    sys.exit(run_pylint())
  File "/Users/they4kman/programming/third-party/halo/.tox/lint/lib/python3.7/site-packages/pylint/__init__.py", line 13, in run_pylint
    Run(sys.argv[1:])
  File "/Users/they4kman/programming/third-party/halo/.tox/lint/lib/python3.7/site-packages/pylint/lint.py", line 1260, in __init__
    'init-hook')))
  File "/Users/they4kman/programming/third-party/halo/.tox/lint/lib/python3.7/site-packages/pylint/lint.py", line 1361, in cb_init_hook
    exec(value) # pylint: disable=exec-used
  File "<string>", line 8
    ZGljdChfX2ZpbGVfXz1hY3RpdmF0ZV90aGlzKSkK'.decode('base64')
                                            ^
SyntaxError: invalid syntax
ERROR: InvocationError: '/Users/they4kman/programming/third-party/halo/.tox/lint/bin/pylint --errors-only --rcfile=/Users/they4kman/programming/third-party/halo/.pylintrc --output-format=colorized halo'

It's been a volcano at work for a hot minute β€” I'm hoping it'll die down, and I'll have Happy Fun Free Timeβ„’ again. I dig your motives, and you've been hella helpful; I'd love to contribute back.

manrajgrover commented 5 years ago

@theY4Kman Yes, that is because linter works on Python 2.7 which this project is mostly written in. It breaks on init-hook for Python 3. Since we can lint only for a particular Python version, I don't think there is a workaround for it but to install Python 2.7.

toejough commented 5 years ago

just encountered this myself. if anyone is unclear how to move forward in the meantime, here's a quick shim:

import contextlib

@contextlib.contextmanager
def halo_shim(text):
    spinner = halo.Halo(text)
    spinner.start()
    try:
        yield spinner
    finally:
        spinner.stop()