fix: improve error handling across the codebase

- cli.py: replace bare except Exception with specific exception types
- cli.py: remove from None to preserve exception chains
- cli.py: remove info cache hint that writes to stderr
- fetcher.py: add attempt count to RuntimeError message
- fetcher.py: use exponential backoff in retry loops
- feed.py: skip malformed RSS items instead of failing entire feed
- download.py: set raise_for_status=True on ClientSession
- mcp.py: wrap search_papers and download_paper in try/except
- config_store.py: warn on config read failure instead of silently ignoring
- config_store.py: handle OSError in write_config

Author: SepineTam

src/nber_cli/cli.py

@@ -388,11 +388,11 @@ def _run_single_download(paper_id: str, output_file: Path | None, save_base: Pat
             downloaded_file = asyncio.run(download_paper_to_file(paper_id, output_file, restrict_dir=restrict_dir))
         else:
             downloaded_file = asyncio.run(download_paper(paper_id, save_base, restrict_dir=restrict_dir))
-    except (Exception, asyncio.CancelledError) as error:
+    except (ClientResponseError, ClientError, TimeoutError, asyncio.CancelledError) as error:
         error_msg = _format_download_error(paper_id, error)
         db.record_download(None, paper_id, "failed", error=error_msg)
         print(error_msg, file=sys.stderr)
-        raise SystemExit(1) from None
+        raise SystemExit(1) from error
 
     db.record_download(None, paper_id, "success", saved_path=str(downloaded_file))
     _print_download_success(paper_id, downloaded_file)
@@ -434,15 +434,6 @@ def _info_payload(paper, include_all: bool) -> dict:
     return result
 
 
-def _print_info_cache_hit_hint(paper_id: int) -> None:
-    normalized_paper_id = f"w{paper_id}"
-    print(
-        "Loaded from info cache. "
-        f"To fetch the latest version, run: nber-cli info {normalized_paper_id} --refresh",
-        file=sys.stderr,
-    )
-
-
 def _feed_clean_options(args: argparse.Namespace) -> dict[str, object]:
     return {
         "days": args.days,
@@ -666,8 +657,6 @@ def main() -> None:
 
         paper = info_cache_result.paper
         db.record_info(None, nber_id)
-        if info_cache_result.from_cache:
-            _print_info_cache_hit_hint(nber_id)
 
         if args.output_format == "json":
             _print_json(_info_payload(paper, args.show_all))

src/nber_cli/config_store.py

@@ -124,7 +124,10 @@ def read_config(config_path: Path | None = None) -> dict[str, Any]:
     else:
         try:
             loaded = json.loads(resolved_path.read_text())
-        except (OSError, json.JSONDecodeError):
+        except (OSError, json.JSONDecodeError) as error:
+            import warnings
+
+            warnings.warn(f"failed to read config from {resolved_path}: {error.__class__.__name__}")
             loaded = None
         config = loaded if isinstance(loaded, dict) else {}
 
@@ -134,8 +137,11 @@ def read_config(config_path: Path | None = None) -> dict[str, Any]:
 
 def write_config(config: dict[str, Any], config_path: Path | None = None) -> None:
     resolved_path = config_path or default_config_path()
-    resolved_path.parent.mkdir(parents=True, exist_ok=True)
-    resolved_path.write_text(json.dumps(config, ensure_ascii=False, indent=2) + "\n")
+    try:
+        resolved_path.parent.mkdir(parents=True, exist_ok=True)
+        resolved_path.write_text(json.dumps(config, ensure_ascii=False, indent=2) + "\n")
+    except OSError as error:
+        raise RuntimeError(f"failed to write config to {resolved_path}: {error.__class__.__name__}") from error
 
 
 def update_database_config(

src/nber_cli/download.py

@@ -85,7 +85,7 @@ async def _fetch(sess: ClientSession) -> bytes:
         connector = _create_connector()
         timeout = ClientTimeout(total=NBER_CLI_CONFIG.request_timeout_seconds)
         async with ClientSession(
-            timeout=timeout, connector=connector, headers=headers
+            timeout=timeout, connector=connector, headers=headers, raise_for_status=True
         ) as base_session:
             content = await _retry_async(lambda: _fetch(base_session))
 

src/nber_cli/feed.py

@@ -121,26 +121,29 @@ def parse_feed_xml(xml_text: str) -> list[NBERFeedItem]:
 
     items: list[NBERFeedItem] = []
     for raw_item in root.findall("./channel/item"):
-        raw_title = _clean_feed_text(raw_item.findtext("title"))
-        title, authors = _parse_feed_title(raw_title)
-        source_url = _clean_feed_text(raw_item.findtext("link"))
-        guid = _clean_feed_text(raw_item.findtext("guid")) or source_url
-        paper_id = _extract_paper_id(source_url) or _extract_paper_id(guid)
-        if not paper_id:
-            raise ValueError("NBER RSS item is missing a paper ID")
-
-        url, _ = urldefrag(source_url)
-        items.append(
-            NBERFeedItem(
-                paper_id=paper_id,
-                title=title,
-                authors=authors,
-                abstract=_clean_feed_text(raw_item.findtext("description")),
-                url=url or source_url,
-                source_url=source_url,
-                guid=guid,
+        try:
+            raw_title = _clean_feed_text(raw_item.findtext("title"))
+            title, authors = _parse_feed_title(raw_title)
+            source_url = _clean_feed_text(raw_item.findtext("link"))
+            guid = _clean_feed_text(raw_item.findtext("guid")) or source_url
+            paper_id = _extract_paper_id(source_url) or _extract_paper_id(guid)
+            if not paper_id:
+                continue
+
+            url, _ = urldefrag(source_url)
+            items.append(
+                NBERFeedItem(
+                    paper_id=paper_id,
+                    title=title,
+                    authors=authors,
+                    abstract=_clean_feed_text(raw_item.findtext("description")),
+                    url=url or source_url,
+                    source_url=source_url,
+                    guid=guid,
+                )
             )
-        )
+        except Exception:
+            continue
 
     return items
 

src/nber_cli/fetcher.py

@@ -118,12 +118,12 @@ def _load_text_sync(url: str) -> str:
                 raise
             last_error = error
 
-        time.sleep(0)
+        time.sleep(min(2 ** attempt, 30))
 
     if last_error is not None:
         raise last_error
 
-    raise RuntimeError("request failed unexpectedly")
+    raise RuntimeError(f"request failed after {NBER_CLI_CONFIG.request_attempts} attempts")
 
 
 async def _retry_async(loader):
@@ -141,12 +141,12 @@ async def _retry_async(loader):
                 raise
             last_error = error
 
-        await asyncio.sleep(0)
+        await asyncio.sleep(min(2 ** attempt, 30))
 
     if last_error is not None:
         raise last_error
 
-    raise RuntimeError("request failed unexpectedly")
+    raise RuntimeError(f"request failed after {NBER_CLI_CONFIG.request_attempts} attempts")
 
 
 def parse_page(page: str) -> NBER:

src/nber_cli/mcp.py

@@ -79,38 +79,44 @@ async def search_papers(
     Returns:
         Dictionary containing result metadata and papers.
     """
-    results = await search_nber(
-        query,
-        start_date=start_date,
-        end_date=end_date,
-        page=page,
-        per_page=per_page,
-    )
-    return search_results(results)
+    try:
+        results = await search_nber(
+            query,
+            start_date=start_date,
+            end_date=end_date,
+            page=page,
+            per_page=per_page,
+        )
+        return search_results(results)
+    except Exception as error:
+        return {"error": f"Search failed: {error.__class__.__name__}"}
 
 
 @nber_mcp.tool()
-async def download_paper(paper_id: str, output_path: Optional[str] = None) -> bool:
+async def download_paper(paper_id: str, output_path: Optional[str] = None) -> dict:
     """Download an NBER working paper as a PDF.
 
     Args:
         paper_id: Paper ID, e.g. 'w1234' or '1234'
         output_path: Explicit output file path, must be within the current directory. If not provided, saves as <paper_id>.pdf in current directory.
 
     Returns:
-        True if download succeeds. On failure, an exception is raised and propagated to the caller.
+        Dictionary with "success": True or "error": message.
     """
-    nber_id = _parse_paper_id(paper_id)
-    normalized_id = f"w{nber_id}"
-
-    if output_path:
-        target = Path(output_path)
-        try:
-            target.absolute().relative_to(Path.cwd().absolute())
-        except ValueError:
-            raise ValueError("MCP download only allows paths within the current directory")
-        await download_paper_to_file(normalized_id, target, restrict_dir=True)
-    else:
-        await download_paper_to_dir(normalized_id, Path.cwd(), restrict_dir=True)
-
-    return True
+    try:
+        nber_id = _parse_paper_id(paper_id)
+        normalized_id = f"w{nber_id}"
+
+        if output_path:
+            target = Path(output_path)
+            try:
+                target.absolute().relative_to(Path.cwd().absolute())
+            except ValueError:
+                return {"error": "MCP download only allows paths within the current directory"}
+            await download_paper_to_file(normalized_id, target, restrict_dir=True)
+        else:
+            await download_paper_to_dir(normalized_id, Path.cwd(), restrict_dir=True)
+
+        return {"success": True}
+    except Exception as error:
+        return {"error": f"Download failed: {error.__class__.__name__}"}

tests/test_cli.py

@@ -639,12 +639,10 @@ def test_info_cache_hit_skips_network(self, mock_get_nber, tmp_path, capsys):
             ),
         )
 
-        with patch("nber_cli.cli._print_info_cache_hit_hint") as mock_hint:
-            with patch.object(sys, "argv", ["nber-cli", "info", "w1234"]):
-                main()
+        with patch.object(sys, "argv", ["nber-cli", "info", "w1234"]):
+            main()
 
         mock_get_nber.assert_not_called()
-        mock_hint.assert_called_once_with(1234)
         captured = capsys.readouterr()
         assert "Cached Title" in captured.out
         with sqlite3.connect(db_path) as connection:
@@ -671,12 +669,10 @@ def test_info_json_cache_hit_keeps_stdout_json(self, mock_get_nber, tmp_path, ca
             ),
         )
 
-        with patch("nber_cli.cli._print_info_cache_hit_hint") as mock_hint:
-            with patch.object(sys, "argv", ["nber-cli", "info", "w1234", "--format", "json"]):
-                main()
+        with patch.object(sys, "argv", ["nber-cli", "info", "w1234", "--format", "json"]):
+            main()
 
         mock_get_nber.assert_not_called()
-        mock_hint.assert_called_once_with(1234)
         captured = capsys.readouterr()
         payload = json.loads(captured.out)
         assert payload["title"] == "Cached Title"

tests/test_mcp.py

@@ -103,18 +103,19 @@ class TestDownloadPaper:
     async def test_download_to_default_path(self):
         with patch("nber_cli.mcp.download_paper_to_dir", new_callable=AsyncMock) as mock_download:
             result = await download_paper("w1234")
-            assert result is True
+            assert result == {"success": True}
             mock_download.assert_called_once()
 
     @pytest.mark.asyncio
     async def test_download_to_explicit_path(self):
         target = Path.cwd() / "paper.pdf"
         with patch("nber_cli.mcp.download_paper_to_file", new_callable=AsyncMock) as mock_download:
             result = await download_paper("w1234", output_path=str(target))
-            assert result is True
+            assert result == {"success": True}
             mock_download.assert_called_once_with("w1234", target, restrict_dir=True)
 
     @pytest.mark.asyncio
     async def test_rejects_path_outside_cwd(self):
-        with pytest.raises(ValueError, match="within the current directory"):
-            await download_paper("w1234", output_path="/tmp/paper.pdf")
+        result = await download_paper("w1234", output_path="/tmp/paper.pdf")
+        assert "error" in result
+        assert "within the current directory" in result["error"]