Surface auth failures when saving a command

[rcaloras]

save_command never checked the response status, so a rejected auth token was swallowed silently and every command was dropped with nothing recorded anywhere.

The bug

requests does not raise on a 4xx, and save_command called requests.post without raise_for_status(). The except Exception block that checks for 401/403 was therefore unreachable for HTTP errors — nothing ever raised to reach it.

Confirmed by mocking a 401 response: the call printed nothing and returned normally. An expired token means the client looks like it is working while saving nothing, indefinitely.

search and get_status_view get this right by accident — their .json() parsing throws on an error body, which reaches the handler. save_command had no such accident.

The change

         r = requests.post(url, data=command.to_JSON(),
                           headers=json_auth_headers())
+        r.raise_for_status()
     except ConnectionError:
         print("Sorry, looks like there's a connection error")
-        pass
     except Exception:
         if r is not None and r.status_code in (403, 401):
             print("Permissions Issue. Run bashhub setup to re-login.")

raise_for_status() makes the existing handler reachable, matching what the other endpoints already do.

5xx stays quiet on purpose. This runs on every shell command, so a transient server error must not write on every prompt. Only 401/403 — which are persistent and actionable — produce output.

Where this actually surfaces

Worth knowing before reviewing: this makes the failure diagnosable in the log, not visible in the terminal.

The shell hook backgrounds the save and redirects it, at lib-bashhub.sh:67:

(__bh_process_command "$command"&) >> "$bashhub_dir"/log.txt 2>&1

__bh_process_command (lines 77–107) is where bashhub save runs, so both stdout and stderr go to ~/.bashhub/log.txt. Fish does the same at bashhub.fish:73. Verified by simulating the exact construct — the message lands in the log and nothing reaches the terminal.

There is a real gap left over. __bh_check_bashhub_installation (lines 112–139) runs once per session outside the redirect and already prints things like "Missing Bashhub access token. Please run 'bashhub setup' to re-login." — but it only greps the config for the presence of access_token, never whether the server still accepts it. So an expired-but-present token falls between the two: the startup check sees a well-formed config and stays quiet, and the per-command failure goes to a log nobody reads.

Closing that would need a separate once-per-session notification (for example, save_command dropping a marker file that the session-start check reads and clears). Deliberately not in this PR — printing on every prompt is not an acceptable alternative.

Tests

Five tests in tests/test_rest_client.py covering 401, 403, success, 5xx-stays-quiet, and connection errors. Reverting raise_for_status fails the 401 and 403 cases, so they are real regression guards.

Note on the test change

tests/test_bashhub.py has a one-line change that is a prerequisite, not a drive-by. test_bashhub_save assigned rest_client.save_command directly and never restored it, leaving the stub live for the rest of the run — the new tests would call the stub instead of the real function and all five would fail. It now uses monkeypatch, which restores it.

That test leaks three bashhub_globals values the same way, and one of those leaks is load-bearing for another test: test_get_bh_filter asserts on bashhub_globals.BH_FILTER, which is computed once at import and so can never reflect a config written during the test — it passes only because it inherits the leaked 'echo'. That is pre-existing on master (it already fails there when run alone or in reverse file order) and is left untouched here to keep this PR to the one fix.

Verification

119 passed, mypy clean, ruff check bashhub/ clean. ruff check tests/ reports 12 findings on both master and this branch — none added.

🤖 Generated with Claude Code