Skip to content

fix: ensure that error logs contain all available details#4183

Merged
johanneskoester merged 2 commits into
snakemake:mainfrom
cosunae:fix_job_log
May 29, 2026
Merged

fix: ensure that error logs contain all available details#4183
johanneskoester merged 2 commits into
snakemake:mainfrom
cosunae:fix_job_log

Conversation

@cosunae

@cosunae cosunae commented May 5, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

The error log ignores the values of the dict passed except the groupid, for example the aux_logs

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).
  • I, as a human being, have checked each line of code in this pull request

AI-assistance disclosure

I used AI assistance for:

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Summary by CodeRabbit

  • Bug Fixes
    • Improved structured error logging to correctly handle metadata in error messages.

@cosunae cosunae requested a review from johanneskoester as a code owner May 5, 2026 14:21
@coderabbitai

coderabbitai Bot commented May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The GroupJob.log_error method in src/snakemake/jobs.py is modified to pass structured logging metadata via the extra= keyword argument to logger.error() instead of as a positional argument, preserving all logged fields while correcting the parameter passing convention.

Changes

Logging Parameter Passing

Layer / File(s) Summary
Logging Implementation
src/snakemake/jobs.py		 GroupJob.log_error now passes the metadata dictionary to logger.error() using the extra= keyword argument instead of as a positional argument, preventing the dict from being misinterpreted as printf formatting arguments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description explains the issue and provides context, but the critical QC checklist items are unchecked, leaving test coverage status uncertain. Confirm whether existing tests cover this logging change, or add new test cases. Also ensure the unchecked QC items reflect the actual state of the PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The PR title accurately describes the main change: fixing error log handling to ensure all dictionary details (like aux_logs) are properly included via the extra= keyword argument.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cosunae cosunae changed the title fix error log fix: error log May 5, 2026
@johanneskoester johanneskoester changed the title fix: error log fix: ensure that error logs contain all available details May 29, 2026
@johanneskoester johanneskoester enabled auto-merge (squash) May 29, 2026 12:27
@johanneskoester johanneskoester merged commit 74a86e9 into snakemake:main May 29, 2026
60 checks passed
@snakemake-bot snakemake-bot mentioned this pull request May 29, 2026
Merged
johanneskoester pushed a commit that referenced this pull request May 29, 2026
@snakemake-bot
chore(main): release 9.21.1 (#4200)
5b5482e 
🤖 I have created a release *beep* *boop*
---


##
[9.21.1](v9.21.0...v9.21.1)
(2026-05-29)


### Bug Fixes

* add default json function to benchmarks
([#4128](#4128))
([41fab22](41fab22))
* do not rerun when checkpoint job missing but downstream file exists
([#4124](#4124))
([a060b93](a060b93))
* ensure that error logs contain all available details
([#4183](#4183))
([74a86e9](74a86e9))
* handle missing pss attribute in benchmark on Windows
([#4160](#4160))
([da52080](da52080))
* implement Resources.setdefault
([#3968](#3968))
([2413e99](2413e99))
* reporting remote nodes number
([#3978](#3978))
([8c534f0](8c534f0))
* resolve pathvars before constructing storage queries
([#3969](#3969))
([bd15237](bd15237))
* storage temp() file cleanup with RemoteProvider
([#4189](#4189))
([898bad1](898bad1))
* tolerate FileNotFoundError in drop_iocache
([#4153](#4153))
([#4191](#4191))
([ce26b28](ce26b28))


### Documentation

* Added guide on debugging workflows
([#4029](#4029))
([3d052ae](3d052ae))
* **cli:** Remove broken ref bold markup
([#4204](#4204))
([1200ebf](1200ebf))
* remove duplicated resources attribute in rules.rst
([#4190](#4190))
([6c8ecdd](6c8ecdd))
* **rules:** Update script type hint advice
([#4193](#4193))
([6108712](6108712))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johanneskoester johanneskoester johanneskoester approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cosunae @johanneskoester