Skip to content

Fix #332: Disable FAIL_ON_EMPTY_BEANS for NF 25.10.x - #333

Merged
drernie merged 4 commits into
mainfrom
332-disable-fail_on_empty_beans
Apr 30, 2026
Merged

Fix #332: Disable FAIL_ON_EMPTY_BEANS for NF 25.10.x#333
drernie merged 4 commits into
mainfrom
332-disable-fail_on_empty_beans

Conversation

@drernie

@drernie drernie commented Apr 30, 2026

Copy link
Copy Markdown
Member

Summary

  • Disable SerializationFeature.FAIL_ON_EMPTY_BEANS on the shared Jackson ObjectMapper so writeMapToPackage.toJson survives Nextflow 25.10.x configs containing dynamic process directive closures (e.g. cpus = { task.attempt * 2 }) that transitively pull in the script binding's SecretsLoader$1 field.
  • Add a regression test in QuiltProductTest that nests an anonymous-class object inside the config map and confirms toJson does not throw.
  • Add make bump [LEVEL=patch|minor|major] (default patch) backed by wf/bump-version.sh, which rewrites version in build.gradle and commits.
  • Bump to 1.0.1, pre-publish nf-quilt/dest-1.0.1 to udp-spec, update CHANGELOG.

Closes #332.

Test plan

  • ./gradlew check passes (test + jacoco coverage gate)
  • Verified the new regression test fails on main's ObjectMapper and passes with FAIL_ON_EMPTY_BEANS disabled
  • make pkg-test WRITE_BUCKET=udp-spec published nf-quilt/dest-1.0.1 successfully

🤖 Generated with Claude Code

Greptile Summary

This PR fixes a serialization crash introduced in Nextflow 25.10.x where dynamic process directive closures (e.g. cpus = { task.attempt * 2 }) transitively pull in SecretsLoader$1 — an anonymous inner class with no bean properties — causing Jackson to throw when toJson is called. The fix disables SerializationFeature.FAIL_ON_EMPTY_BEANS on the shared ObjectMapper, accompanied by a targeted regression test, a make bump convenience target, and a version bump to 1.0.1.

Confidence Score: 4/5

Safe to merge; the core fix is minimal and correct, with only a trivial style issue in the new shell script.

All P2-only findings — the sole issue is an unescaped dot in the sed regex inside bump-version.sh, which is extremely unlikely to cause a false match in practice. The main bug fix and regression test are correct and well-scoped.

wf/bump-version.sh (minor sed escaping issue on line 29)

Important Files Changed

Filename Overview
src/main/groovy/nextflow/quilt/QuiltProduct.groovy Adds .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) to the shared ObjectMapper — correct targeted fix for NF 25.10.x closure serialization failure.
src/test/groovy/nextflow/quilt/QuiltProductTest.groovy Adds regression test verifying toJson handles anonymous inner classes without throwing; assertion and Spock structure are correct.
wf/bump-version.sh New version-bump helper script; minor: version string used unescaped as a sed regex, so dots act as wildcards (unlikely to cause real problems but not strictly correct).
Makefile Adds bump phony target delegating to wf/bump-version.sh with configurable LEVEL; straightforward and correct.
build.gradle Version bump from 1.0.0 to 1.0.1 — matches CHANGELOG and PR intent.
CHANGELOG.md 1.0.1 release notes added with accurate description of the fix and new make bump target.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["NF 25.10.x config map\n(contains dynamic closures)"] --> B["QuiltProduct.writeMapToPackage"]
    B --> C["toJson(map)"]
    C --> D["OBJECT_WRITER.writeValueAsString(value)"]
    D --> E{FAIL_ON_EMPTY_BEANS?}
    E -- "enabled (main)" --> F["JsonMappingException\nSecretsLoader$1 has no properties"]
    E -- "disabled (this PR)" --> G["Serializes empty bean as {}"]
    G --> H["JSON string written to package"]
Loading

Reviews (1): Last reviewed commit: "Update CHANGELOG for 1.0.1" | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

drernie and others added 4 commits April 30, 2026 15:12
Dynamic process directive closures (e.g. cpus = { task.attempt * 2 })
transitively pull in the script binding's secrets field, an anonymous
SecretsLoader$1 with no bean properties. Jackson would throw when
serializing config/params/workflow JSON.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@drernie drernie linked an issue Apr 30, 2026 that may be closed by this pull request
3 tasks
@drernie
drernie merged commit 9cf5d68 into main Apr 30, 2026
10 checks passed
@drernie
drernie deleted the 332-disable-fail_on_empty_beans branch April 30, 2026 22:34
Comment thread wf/bump-version.sh
esac
NEW="${MAJOR}.${MINOR}.${PATCH}"

sed -i.bak "s/^version = '${OLD}'\$/version = '${NEW}'/" "$FILE"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Unescaped dots in sed regex pattern

${OLD} (e.g. 1.0.1) is inserted directly into a sed regex, where . is a wildcard matching any character. In practice the surrounding single-quote context makes a false match extremely unlikely, but it is safer to escape the dots so the pattern is anchored to literal version strings.

Suggested change
sed -i.bak "s/^version = '${OLD}'\$/version = '${NEW}'/" "$FILE"
sed -i.bak "s/^version = '${OLD//./\\.}'\$/version = '${NEW}'/" "$FILE"

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.

Disable FAIL_ON_EMPTY_BEANS

1 participant