Skip to content

fix: raise CompileError when solc returns no contracts to avoid batch crash#160

Merged
TheDZhon merged 2 commits into
mainfrom
fix/compile-error-batch-fragility
May 21, 2026
Merged

fix: raise CompileError when solc returns no contracts to avoid batch crash#160
TheDZhon merged 2 commits into
mainfrom
fix/compile-error-batch-fragility

Conversation

@tamtamchik
Copy link
Copy Markdown
Member

Summary

  • compile_contracts() returned json.loads(stdout) without checking for "contracts". When solc returned an errors-only response (e.g. unresolved import from version drift in a dependency), compile_contract_from_explorer() did compile_contracts(...)["contracts"] and raised KeyError.
  • KeyError is not a BaseCustomException, so it bypassed both the per-contract and outer handlers in process_config, killing the whole batch — one bad implementation address took down all remaining contracts in the config.
  • Fix: compile_contracts() now raises a structured CompileError with formatted solc error messages when output has no "contracts" key, and also when stdout is non-JSON. The existing except BaseCustomException in process_config then catches it and continues with the next contract.

Test plan

  • uv run pytest -q — 92 passed
  • New tests/test_compiler.py covers: errors-only output, warning-only filtering, invalid JSON, happy path
  • uv run black --check diffyscan/utils/compiler.py tests/test_compiler.py

@tamtamchik tamtamchik requested a review from a team as a code owner May 20, 2026 16:46
@tamtamchik tamtamchik requested a review from Copilot May 20, 2026 16:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Solidity compilation error handling so that when solc --standard-json returns an errors-only (or otherwise malformed) response, the tool raises a structured CompileError instead of allowing a downstream KeyError to crash batch processing.

Changes:

  • Parse solc stdout defensively in compile_contracts() and raise CompileError for non-JSON output or missing "contracts".
  • Surface formatted solc error messages (filtering warnings when errors exist) in the raised exception.
  • Add unit tests covering errors-only output, warning filtering, invalid JSON, and success cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
diffyscan/utils/compiler.py Adds safer JSON parsing and explicit CompileError paths when solc output is malformed or missing contracts.
tests/test_compiler.py Introduces pytest coverage for the new compile_contracts() error-handling behavior.
Comments suppressed due to low confidence (1)

diffyscan/utils/compiler.py:123

  • This branch assumes output is a dict (uses output.get(...)), but if solc (or a wrapper) returns valid JSON that isn't an object (e.g., null, list, or string), this will raise AttributeError and bypass CompileError. Consider validating isinstance(output, dict) before checking keys / calling .get, and raising CompileError with a clear diagnostic when the top-level JSON shape is unexpected.
    if "contracts" not in output:
        errors = output.get("errors") or []
        fatal = [e for e in errors if e.get("severity") == "error"] or errors
        msgs = "\n".join(
            e.get("formattedMessage") or e.get("message") or str(e) for e in fatal
        )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread diffyscan/utils/compiler.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@TheDZhon TheDZhon merged commit ac96e86 into main May 21, 2026
13 checks passed
@TheDZhon TheDZhon deleted the fix/compile-error-batch-fragility branch May 21, 2026 10:14
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.

3 participants