Skip to content

Convert configuration error printf calls to logError (#412) - #417

Merged
Alomir merged 6 commits into
PecanProject:masterfrom
coder-Yash886:fix/412-config-logging
Oct 26, 2026
Merged

Alomir merged 6 commits into
PecanProject:masterfrom
coder-Yash886:fix/412-config-logging

Conversation

@coder-Yash886

Copy link
Copy Markdown
Contributor

closes#412

Summary

  • What: Replaced raw printf() configuration error calls in src/common/context.c with SIPNET's logging system (logError()), ensuring consistent error prefixes and logging levels. Also made error messages actionable by identifying offending parameter names and types where available.
  • Motivation: Resolves Use consistent logging for configuration errors #412. Some configuration error checks in context.c used raw printf() while others used logError(), resulting in inconsistent error reporting across configuration failures and bypassing log level handling.

How was this change tested?

  1. Ran full local test suite:
    • make test — all 24 unit tests passed and all 5 smoke test suites passed with zero diffs.
  2. Verified error visibility with --quiet:
    • Ran ./sipnet -i tests/smoke/niwot/sipnet.in --quiet --soil-phenol --gdd and confirmed [ERROR ] soil-phenol and gdd may not both be turned on remains visible (exit code 3).
    • Ran ./sipnet -i tests/smoke/niwot/sipnet.in --quiet --file-prefix "" and confirmed [ERROR ] filePrefix must be set for SIPNET to run remains visible (exit code 3).

Reproduction steps

Run SIPNET with conflicting or invalid configuration options using --quiet:

./sipnet -i tests/smoke/niwot/sipnet.in --quiet --soil-phenol --gdd

@coder-Yash886

Copy link
Copy Markdown
Contributor Author

@dlebauer @Alomir @infotroph Please Review the Pr when you have free time.

@Alomir Alomir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @coder-Yash886, thank you for contributing! A couple of comments:

  1. These should be calls to logInternalError, not logError
  2. Please remove the Internal error: text from the log lines, as that is printed as part of logInternalError

@Alomir

Alomir commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Whoops, that's not clear enough: the calls that exit with EXIT_CODE_INTERNAL_ERROR should use logInternalError - the others should be logError

Comment thread src/common/context.c Resolved
@coder-Yash886

Copy link
Copy Markdown
Contributor Author

Hi @Alomir @dlebauer, updated to logInternalError and removed the Internal error: prefix as requested. Tests pass. Ready for review!

@Alomir Alomir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great, thanks!

@Alomir
Alomir merged commit 6c21934 into PecanProject:master Oct 26, 2026
11 checks passed
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.

Use consistent logging for configuration errors

2 participants