Use better error message handling in the command-line interface
Marco Ricci

Marco Ricci commited on 2024-08-16 12:38:08
Zeige 2 geänderte Dateien mit 30 Einfügungen und 19 Löschungen.


Instead of blindly relying on `click.Context.fail`, which causes a usage
message to be printed, implement our own error-exit function.

Furthermore, since writing the configuration file is prone to similar
problems as reading it, include a wrapper for writing the configuration
file as well, which ensures errors are properly reported.

Due to the rewrite, some error messages changed slightly in phrasing
and/or in capitalization.
... ...
@@ -16,6 +16,7 @@ import os
16 16
 import socket
17 17
 from typing import (
18 18
     TYPE_CHECKING,
19
+    NoReturn,
19 20
     TextIO,
20 21
     cast,
21 22
 )
... ...
@@ -844,13 +845,23 @@ def derivepassphrase(
844 845
                     opt_str, f'mutually exclusive with {other_str}', ctx=ctx
845 846
                 )
846 847
 
848
+    def err(msg: str) -> NoReturn:
849
+        click.echo(f'{PROG_NAME}: {msg}', err=True)
850
+        ctx.exit(1)
851
+
847 852
     def get_config() -> _types.VaultConfig:
848 853
         try:
849 854
             return _load_config()
850 855
         except FileNotFoundError:
851 856
             return {'services': {}}
852 857
         except Exception as e:  # noqa: BLE001
853
-            ctx.fail(f'cannot load config: {e}')
858
+            err(f'Cannot load config: {e}')
859
+
860
+    def put_config(config: _types.VaultConfig, /) -> None:
861
+        try:
862
+            _save_config(config)
863
+        except Exception as exc:  # noqa: BLE001
864
+            err(f'Cannot store config: {exc}')
854 865
 
855 866
     configuration: _types.VaultConfig
856 867
 
... ...
@@ -914,24 +925,24 @@ def derivepassphrase(
914 925
                     break
915 926
             else:
916 927
                 if not notes_value.strip():
917
-                    ctx.fail('not saving new notes: user aborted request')
928
+                    err('not saving new notes: user aborted request')
918 929
             configuration['services'].setdefault(service, {})['notes'] = (
919 930
                 notes_value.strip('\n')
920 931
             )
921
-            _save_config(configuration)
932
+            put_config(configuration)
922 933
     elif delete_service_settings:
923 934
         assert service is not None
924 935
         configuration = get_config()
925 936
         if service in configuration['services']:
926 937
             del configuration['services'][service]
927
-            _save_config(configuration)
938
+            put_config(configuration)
928 939
     elif delete_globals:
929 940
         configuration = get_config()
930 941
         if 'global' in configuration:
931 942
             del configuration['global']
932
-            _save_config(configuration)
943
+            put_config(configuration)
933 944
     elif clear_all_settings:
934
-        _save_config({'services': {}})
945
+        put_config({'services': {}})
935 946
     elif import_settings:
936 947
         try:
937 948
             # TODO: keep track of auto-close; try os.dup if feasible
... ...
@@ -943,13 +954,13 @@ def derivepassphrase(
943 954
             with infile:
944 955
                 maybe_config = json.load(infile)
945 956
         except json.JSONDecodeError as e:
946
-            ctx.fail(f'Cannot load config: cannot decode JSON: {e}')
957
+            err(f'Cannot load config: cannot decode JSON: {e}')
947 958
         except OSError as e:
948
-            ctx.fail(f'Cannot load config: {e.strerror}')
959
+            err(f'Cannot load config: {e.strerror}: {e.filename!r}')
949 960
         if _types.is_vault_config(maybe_config):
950
-            _save_config(maybe_config)
961
+            put_config(maybe_config)
951 962
         else:
952
-            ctx.fail('not a valid config')
963
+            err(f'Cannot load config: {_INVALID_VAULT_CONFIG}')
953 964
     elif export_settings:
954 965
         configuration = get_config()
955 966
         try:
... ...
@@ -962,7 +973,7 @@ def derivepassphrase(
962 973
             with outfile:
963 974
                 json.dump(configuration, outfile)
964 975
         except OSError as e:
965
-            ctx.fail(f'cannot write config: {e.strerror}')
976
+            err(f'Cannot store config: {e.strerror}: {e.filename!r}')
966 977
     else:
967 978
         configuration = get_config()
968 979
         # This block could be type checked more stringently, but this
... ...
@@ -1001,13 +1012,13 @@ def derivepassphrase(
1001 1012
                     'ASCII'
1002 1013
                 )
1003 1014
             except IndexError:
1004
-                ctx.fail('no valid SSH key selected')
1015
+                err('no valid SSH key selected')
1005 1016
             except (LookupError, RuntimeError) as e:
1006
-                ctx.fail(str(e))
1017
+                err(str(e))
1007 1018
         elif use_phrase:
1008 1019
             maybe_phrase = _prompt_for_passphrase()
1009 1020
             if not maybe_phrase:
1010
-                ctx.fail('no passphrase given')
1021
+                err('no passphrase given')
1011 1022
             else:
1012 1023
                 phrase = maybe_phrase
1013 1024
         if store_config_only:
... ...
@@ -609,7 +609,7 @@ class TestCLI:
609 609
                 result.stderr_bytes
610 610
             ), 'program did not print any error message'
611 611
             assert (
612
-                b'not a valid config' in result.stderr_bytes
612
+                b'Invalid vault config' in result.stderr_bytes
613 613
             ), 'program did not print the expected error message'
614 614
 
615 615
     def test_213a_import_bad_config_not_json_data(
... ...
@@ -699,7 +699,7 @@ class TestCLI:
699 699
                 result.stderr_bytes
700 700
             ), 'program did not print any error message'
701 701
             assert (
702
-                b'cannot load config' in result.stderr_bytes
702
+                b'Cannot load config' in result.stderr_bytes
703 703
             ), 'program did not print the expected error message'
704 704
 
705 705
     def test_214b_export_settings_not_a_file(
... ...
@@ -724,7 +724,7 @@ class TestCLI:
724 724
                 result.stderr_bytes
725 725
             ), 'program did not print any error message'
726 726
             assert (
727
-                b'cannot load config' in result.stderr_bytes
727
+                b'Cannot load config' in result.stderr_bytes
728 728
             ), 'program did not print the expected error message'
729 729
 
730 730
     def test_214c_export_settings_target_not_a_file(
... ...
@@ -747,7 +747,7 @@ class TestCLI:
747 747
                 result.stderr_bytes
748 748
             ), 'program did not print any error message'
749 749
             assert (
750
-                b'cannot write config' in result.stderr_bytes
750
+                b'Cannot store config' in result.stderr_bytes
751 751
             ), 'program did not print the expected error message'
752 752
 
753 753
     def test_214d_export_settings_settings_directory_not_a_directory(
... ...
@@ -773,7 +773,7 @@ class TestCLI:
773 773
                 result.stderr_bytes
774 774
             ), 'program did not print any error message'
775 775
             assert (
776
-                b'cannot load config' in result.stderr_bytes
776
+                b'Cannot load config' in result.stderr_bytes
777 777
             ), 'program did not print the expected error message'
778 778
 
779 779
     def test_220_edit_notes_successfully(self, monkeypatch: Any) -> None:
780 780