Skip to content

Propagate errors instead of swallowing them - #27

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1784930852-error-handling
Open

Propagate errors instead of swallowing them#27
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1784930852-error-handling

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Several failure paths were silently absorbed, so a broken run could still look like a clean report. This makes each of them either propagate or become visible (logged + surfaced in the CLI/report), without changing the happy path.

The worst offender: a file that failed to download was handed to the LLM as if it were source code.

-            except requests.HTTPError as e:
-                files[file_path] = f"# Error fetching file: {e}"   # analyzed as Go/C#/Java source
+            except requests.RequestException as e:                 # also covers timeouts/connection resets
+                outcome.failures[file_path] = str(e)               # file omitted, caller told about it

CodeFetcher.fetch_eip_files() is the new entrypoint returning FetchOutcome(files, failures) and raising CodeFetchError when nothing could be fetched; fetch_eip_implementation() keeps its old Dict[str, str] signature (successes only). The CLI prints one warning per skipped file.

Other silent paths now surfaced:

  • SpecFetcher.fetch_eip_spec collected warnings (printed by the CLI) and raises SpecFetchError when an EIP has registered spec sources but none could be fetched — previously the analysis quietly ran on EIP prose alone. Only HTTPError/ConnectionError were caught before; timeouts escaped as unhandled crashes.
  • Analyzer backends still return an ERROR AnalysisResult per file (that resilience is deliberate), but now logger.exception the cause and carry error="HTTPError: 500" in to_dict(). Gemini returning response.text is None is treated as a failure rather than crashing inside the parser.
  • _parse_json_response logs when it cannot recover JSON, and handles an empty response explicitly.
  • Report/differential summaries count failed_files and report ANALYSIS FAILED when every file errored — that used to be folded into UNCERTAIN, i.e. a failed run presented as an inconclusive one. prspec analyze warns per failed file and exits non-zero if all of them failed.
  • verifier._one_round and DifferentialEngine.synthesize kept their except Exception fallbacks (they must not abort a run) but no longer discard the exception; a round backed by an ERROR result votes unsure instead of confirm/refute.
  • Cache reads/writes in both fetchers are best-effort: an unreadable/unwritable cache entry logs a warning and falls back to the network rather than aborting the fetch.
  • Config._load_config raises ConfigError on unreadable/invalid YAML or a non-mapping document, instead of failing later with AttributeError: 'NoneType' object has no attribute 'get'.
  • scan_path reports skipped files (summary.files_skipped, errors[]) instead of except OSError: continue.
  • prspec fetch-spec|list-files|list-eips|clear-cache|check-config printed an error and exited 0; they now abort with a non-zero status. Same for run_demo.py, which returned early on failure and exited 0.
  • Narrowed over-broad handlers (except Exception around imports/os.path.relpath), added raise ... from e on re-raised ImportErrors, and guarded raise last_exc when last_exc is None in the Azure retry loop.

New src/errors.py holds PRSpecError and its subclasses (ConfigError, SpecFetchError, CodeFetchError, AnalysisError). CLI commands call logging.basicConfig so the new warnings actually reach stderr (-v switches to DEBUG).

Testing

tests/test_error_handling.py (20 new tests) covers fetch failures being reported rather than inlined, CodeFetchError/SpecFetchError/ConfigError, unreadable-cache fallback, ANALYSIS FAILED summaries, verifier logging, and scan_path skip reporting. Full suite: 169 passed, 2 skipped; ruff check src/ tests/ run_demo.py clean.

Link to Devin session: https://app.devin.ai/sessions/528d8fa6acfc4ddc88746ca656df33e4
Requested by: @Fosurero

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@Fosurero Fosurero self-assigned this Jul 24, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants