Fix source add --type file incorrectly showing auth error for missing files (#154)
The `with_client` decorator caught all `FileNotFoundError` exceptions and treated them as authentication errors. When `source add --type file` was used with a non-existent file, the `FileNotFoundError` from `add_file()` was caught by this broad handler, showing "Not logged in" instead of the actual file-not-found error. Narrowed the `FileNotFoundError` catch to only wrap the auth step (`get_auth_tokens`), so file-not-found errors from command logic are properly propagated as regular errors. Fixes #153 https://claude.ai/code/session_01WsnjDnXMBz76e6sNRjqXGH Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
6a6e1a9484
commit
3ca046b6bc
2 changed files with 36 additions and 4 deletions
|
|
@ -469,14 +469,16 @@ def with_client(f):
|
|||
return elapsed
|
||||
|
||||
try:
|
||||
auth = get_auth_tokens(ctx)
|
||||
try:
|
||||
auth = get_auth_tokens(ctx)
|
||||
except FileNotFoundError:
|
||||
log_result("failed", "not authenticated")
|
||||
handle_auth_error(json_output)
|
||||
return # unreachable (handle_auth_error raises SystemExit), but keeps mypy happy
|
||||
coro = f(ctx, *args, client_auth=auth, **kwargs)
|
||||
result = run_async(coro)
|
||||
log_result("completed")
|
||||
return result
|
||||
except FileNotFoundError:
|
||||
log_result("failed", "not authenticated")
|
||||
handle_auth_error(json_output)
|
||||
except Exception as e:
|
||||
log_result("failed", str(e))
|
||||
if json_output:
|
||||
|
|
|
|||
|
|
@ -447,6 +447,36 @@ class TestWithClientDecorator:
|
|||
assert result.exit_code == 1
|
||||
assert "login" in result.output.lower()
|
||||
|
||||
def test_decorator_file_not_found_in_command_not_treated_as_auth_error(self):
|
||||
"""Test that FileNotFoundError from command logic is NOT treated as auth error.
|
||||
|
||||
Regression test for GitHub issue #153: `source add --type file` with a
|
||||
missing file was incorrectly showing 'Not logged in' because the
|
||||
with_client decorator caught all FileNotFoundError as auth errors.
|
||||
"""
|
||||
import click
|
||||
from click.testing import CliRunner
|
||||
|
||||
@click.command()
|
||||
@with_client
|
||||
def test_cmd(ctx, client_auth):
|
||||
async def _run():
|
||||
raise FileNotFoundError("File not found: /tmp/nonexistent.pdf")
|
||||
|
||||
return _run()
|
||||
|
||||
runner = CliRunner()
|
||||
with patch("notebooklm.cli.helpers.load_auth_from_storage") as mock_load:
|
||||
mock_load.return_value = {"SID": "test"}
|
||||
with patch("notebooklm.cli.helpers.fetch_tokens", new_callable=AsyncMock) as mock_fetch:
|
||||
mock_fetch.return_value = ("csrf", "session")
|
||||
result = runner.invoke(test_cmd)
|
||||
|
||||
assert result.exit_code == 1
|
||||
# Should show the actual file error, NOT an auth error
|
||||
assert "File not found" in result.output
|
||||
assert "login" not in result.output.lower()
|
||||
|
||||
def test_decorator_handles_exception_non_json(self):
|
||||
"""Test error handling in non-JSON mode"""
|
||||
import click
|
||||
|
|
|
|||
Loading…
Reference in a new issue