Wrap creation of zuliprc in try to catch PermissionError.

[fredrikekre]

Previously PermissionErrors where caught in the outer try in the while loop in
parse_zuliprc. This resulted in a misleading error message about invalid
credentials. This patch catches PermissionError directly at the source, and
exits directly since there is no point to keep asking the user for credentials.
In particular, when running the default docker configuration, this happens
since the zulip user in the container does not have permission to create files
in the mapped host folder. Fixes #797.

[sumanthvrao]

@fredrikekre Thanks for debugging this. It looks like we hadn't thought about this permission issue when creating zuliprc files in mapped volumes. (It might be helpful if we add a note of running with -u root in docker/README)

There's one lint error in the PR which is causing CI to fail on Travis (Travis message says it's a line length exceeded issue). You can run our linters locally ./tools/lint-all to reproduce them - assuming you have set up a dev env;

@neiljp Will follow up the work done previously on #571 👍 .

[fredrikekre]

There's one lint error in the PR which is causing CI to fail

Should be fixed now.

It might be helpful if we add a note of running with -u root in docker/README

I assume the zulip user is created in the image such that it does not run as root, so I don't think it should be recommended. Perhaps a note about just creating the file before starting the image (e.g. by downloading it from the web app) can be added though.

[neiljp]

@fredrikekre This looks broadly good 👍 How did you test this? We have some tests for run.py but we could add one for this too.

[fredrikekre]

I just ran something like

$ HOME=/permissionerror zulip-term

and put in the credentials. It should be possible to add a test by calling fetch_zuliprc after mocking relevant functions ('zulipterminal.cli.run.styled_input, zulipterminal.cli.run.get_login_id and builtins.open to throw a PermissionError), however, I don't think I can do it since my experience with pytest in particular and python in general is lacking.

[neiljp]

@fredrikekre I have working test cases (via main) for bad permissions and a bad location (ie. inaccessible valid location and a non-existent location), which both seem applicable and confirm your solution. However, I'm currently going to wait on an issue with pytest failing to garbage-collect inaccessible nested directories, filed as pytest-dev/pytest#7821. That is likely to be able to worked around in the test, but it would be good to get fixed properly if possible.

[neiljp]

@fredrikekre I've used the approach suggested in the issue, resulting in https://github.com/neiljp/zulip-terminal/commits/801_with_tests I think this covers the likely cases which you were trying to fix?

Your commit does clarify the message resulting from #797, which is great, but I'm not sure if it resolves the fundamental issue - ie. that the user ids/names don't match and so the fresh zuliprc cannot be written. Do you agree?

[fredrikekre]

Yea, feel free to push to this branch.

Your commit does clarify the message resulting from #797, which is great, but I'm not sure if it resolves the fundamental issue - ie. that the user ids/names don't match and so the fresh zuliprc cannot be written. Do you agree?

Sure, but with a correct error message I would instantly understand what the issue was and be able to resolve it (most docker users run into these permission errors sometimes I assume, and knows how to fix it). I can submit a follow up pull request adding some more detailed information about this to the docker/README.md file.

[neiljp]

@fredrikekre Given the apparent consensus, I was planning to just rebase/merge that branch onto master, unless you had other plans? We'd certainly welcome a follow-up for folks running docker to clarify this issue.

(that is, push my branch with your commit and the tests onto master, rather than adding the tests to the branch for this PR)

[fredrikekre]

Sure.

[neiljp]

@fredrikekre Just to confirm this was merged - thanks for the fix 👍