Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blanket "except Exception" obfuscating server-specific parsing problems #144

Closed
spolloni opened this issue Oct 27, 2021 · 3 comments
Closed

Comments

@spolloni
Copy link

Hello and thanks for maintaining this useful package!

I am experiencing an issue very similar to this one, where due to a server intricacy, aioftp is failing to list and parse the host's root directory.

It took me a long while to figure this out, because the following line:

except Exception:

prevents the much more useful exception below from being raised:

raise ValueError("All parsers failed to parse", b, ex)

As an example, the following code:

import asyncio
import aioftp

async def main():
    async with aioftp.Client.context('ftp.kymartian.ky.gov') as client:
        print(await client.list())

asyncio.run(main())

will NOT fail and simply print [], which seems like an undesirable behavior. If the more informative ValueError exception was raised instead, it would have been much clearer that using the parse_list_line_custom argument was necessary.

Is there a specific reason for using very generic try/except here?

@spolloni spolloni changed the title blanket "except Exception" obfuscating server-specific parsing problems Blanket "except Exception" obfuscating server-specific parsing problems Oct 27, 2021
@pohmelie
Copy link
Collaborator

AFAIR, try/except was introduced for windows ftp servers support. Since they can return dir command output. What is your suggestion in this case?

@spolloni
Copy link
Author

spolloni commented Dec 7, 2021

Since they can return dir command output.

Sorry I don't follow. How would this be affected by removing the try/except ?

@pohmelie
Copy link
Collaborator

Sorry for a delay. I got your point and agreed, it's better not to shadow errors. Changed in 2e0c317

Will release right after resolve one more issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants