fix(app): return InitChainer errors instead of panics

[namedfarouk]

Summary

• replace panic calls in InitChainer with explicit wrapped errors
• preserve function contract by returning an error from InitChainer on failures
• add a regression test that invalid genesis JSON returns an error without panicking

Testing

• go test -tags=test ./app -run TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON -count=1 -v -timeout 10m
• go test -tags=test ./app -run TestKiichainApp_ -count=1 -timeout 10m

Closes #264

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3675295e-fe48-4839-b7a7-c58456c49efa

📥 Commits

Reviewing files that changed from the base of the PR and between 2dea1e1 and f3aef40.

📒 Files selected for processing (1)

• app/app_test.go

✅ Files skipped from review due to trivial changes (1)

• app/app_test.go

---

Walkthrough

The PR changes KiichainApp.InitChainer (app/app.go) to return errors instead of calling panic for three failure cases: unmarshaling req.AppStateBytes into GenesisState, UpgradeKeeper.SetModuleVersionMap, and app.mm.InitGenesis. Each failure now returns nil plus a wrapped fmt.Errorf. A unit test (app/app_test.go) was added to assert InitChainer does not panic and returns an error when given invalid JSON in AppStateBytes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately summarizes the main change: replacing panic calls with error returns in the InitChainer function.
Description check	✅ Passed	The description is related to the changeset, covering the panic-to-error replacement, function contract preservation, test addition, and test commands.
Linked Issues check	✅ Passed	The code changes fully address all objectives from issue #264: three panic calls are replaced with error returns, the function contract is preserved, and a regression test is added.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue #264 objectives and there are no out-of-scope modifications.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

app/app_test.go (1)

54-67: Assert nil response on error to fully lock the InitChainer contract.

Line 63 currently captures only err. Since this PR’s contract is “nil response + error on failure,” assert resp is nil too.

Suggested test hardening

 func TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON(t *testing.T) {
 	app := kiihelpers.Setup(t)

 	req := &abci.RequestInitChain{
 		AppStateBytes: []byte("{ invalid json }"),
 	}

-	var err error
+	var (
+		resp *abci.ResponseInitChain
+		err  error
+	)
 	require.NotPanics(t, func() {
-		_, err = app.InitChainer(app.NewContext(false), req)
+		resp, err = app.InitChainer(app.NewContext(false), req)
 	})
+	require.Nil(t, resp)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "failed to unmarshal genesis state")
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/app_test.go` around lines 54 - 67, The test
TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON should also assert
that the response from app.InitChainer is nil when an error occurs; change the
call to capture the response (resp, err = app.InitChainer(...)) and add
require.Nil(t, resp) after the NotPanics block (before or alongside
require.Error(t, err)) to enforce the "nil response + error" contract for
InitChainer on invalid genesis JSON; reference app.InitChainer and the
TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON test to locate and
update the assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/app_test.go`:
- Around line 54-67: The test
TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON should also assert
that the response from app.InitChainer is nil when an error occurs; change the
call to capture the response (resp, err = app.InitChainer(...)) and add
require.Nil(t, resp) after the NotPanics block (before or alongside
require.Error(t, err)) to enforce the "nil response + error" contract for
InitChainer on invalid genesis JSON; reference app.InitChainer and the
TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON test to locate and
update the assertion.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8e774430-d5ff-4aa7-a06f-7d3cb64551af

📥 Commits

Reviewing files that changed from the base of the PR and between 988d05f and 2dea1e1.

📒 Files selected for processing (2)

• app/app.go
• app/app_test.go

[namedfarouk]

@coderabbitai addressed, thanks.

Implemented the suggested hardening in TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON by capturing resp, err = app.InitChainer(...) and asserting require.Nil(t, resp) alongside the error assertions.

Validation run:

• go test -tags=test ./app -run TestKiichainApp_InitChainerReturnsErrorForInvalidGenesisJSON -count=1 -v -timeout 10m

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@namedfarouk: Great work! Capturing both resp and err and asserting require.Nil(t, resp) is exactly the right hardening — it ensures the test validates the full function contract (*abci.ResponseInitChain, error) rather than just the error path. Thanks for running the validation test as well! 🎉

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

[jhelison]

This PR is a copy from #309