Database: Fix NullReferenceException in ExecuteNonQuery with ContinueOnError=True

[viogroza]

Summary

• ExecuteNonQuery with ContinueOnError="True" threw System.NullReferenceException instead of swallowing the SQL error, defeating the purpose of ContinueOnError.
• Root cause: on failure, ConnectionHelper.HandleException swallows the exception but leaves affectedRecords null; the result-building action then unconditionally read affectedRecords.Result. ExecuteQuery.cs already guards against this (if (affectedRecords == null) return;) but ExecuteNonQuery.cs was missing the equivalent guard.
• Added the same guard to ExecuteNonQuery.cs.

Test plan

• Added ExecuteNonQuery_ContinueOnError_SwallowsExceptionInsteadOfThrowingNullReferenceException to SqliteIntegrationTests.cs, reproducing the exact repro from the ticket (SELECT * FROM ThisTableDoesNotExist_Probe with ContinueOnError=True). Verified it throws the reported NullReferenceException without the fix, and passes with it.
• Full Activities.Database.sln test suite passes (50/50).
• Validated end-to-end against a real UiPath project (SQLite-backed workflow): reproduces the NullReferenceException against the published GA 2.1.1 package, and resolves cleanly against the fixed build.

Fixes STUD-81504: https://uipath.atlassian.net/browse/STUD-81504

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[Copilot]

🟢 Approval recommended

The guard matches the established ExecuteQuery behavior and the regression test covers the reported failure.

Pull request overview

Fixes ExecuteNonQuery so ContinueOnError=True suppresses SQL failures without triggering a secondary NullReferenceException.

Changes:

• Guards result processing when execution produced no result.
• Adds a SQLite regression test validating default output behavior.

File summaries

File	Description
Activities/Database/UiPath.Database.Activities/ExecuteNonQuery.cs	Skips result handling after a swallowed execution failure.
Activities/Database/UiPath.Database.Tests/SqliteIntegrationTests.cs	Tests invalid SQL with ContinueOnError=True.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[alexandru-petre]

Reviewed the branch against STUD-81504. The fix is the guard the ticket prescribed, in the file it named, and the new test drives the reported repro, so the stated intent and the implementation agree and I found nothing wrong with the change itself.

Two comments below, the first one would require a decision before the merge. Both are about output semantics that neither the ticket nor the ContinueOnError property documentation ever states, rather than about defects in the fix.

[alexandru-petre]

The guard removes the crash, but it also means that on the swallowed path neither AffectedRecords.Set(...) nor ConnectionHelper.SetOutputParameters(...) runs. In Workflow Foundation an OutArgument writes into a workflow variable only when the activity calls Set; if the activity never calls it, the variable is not reset, it keeps the value it already held. So in a loop where iteration 1 updated 5 rows and iteration 2's statement fails with ContinueOnError = True, AffectedRecords still reads 5, and a downstream If AffectedRecords > 0 treats the failed statement as a success. The same applies to the Out and InOut entries in Parameters, which stored procedure callers read back after execution.

The other three activities in this pack behave the opposite way: InsertDataTable, BulkInsert and BulkUpdate hold the row count in a plain int or long local initialised to 0, so after a swallowed error they explicitly write 0 to an identically named and identically typed output. The property description ("Specifies whether the automation should continue even when the activity throws an error") says nothing about output values, so there is no documented contract to appeal to either way. That is exactly why it is worth choosing deliberately rather than inheriting whichever shape was copied.

Both choices are defensible, and mirroring ExecuteQuery is what the ticket suggested, so keeping this as it stands needs nothing more than a sentence in the PR description recording that the stale value is intended. If you would rather match the sibling activities, initialising affectedRecords to new DBExecuteCommandResult() instead of null yields Result == 0 and an empty ParametersBind, so both existing calls stay on the happy path and the guard is no longer needed.

[alexandru-petre]

With the guard in place nothing ever writes this entry, so the 0 comes from the out argument's default initialisation inside WorkflowInvoker rather than from anything the activity did. The assertion passes identically whether the activity writes 0 or writes nothing at all.

The test is still a genuine regression test: without the fix Invoke throws NullReferenceException and the test fails before reaching this line, so the verification that matters does happen. The issue is that this line reads as though it pins down an output contract when it does not, and the "writes nothing" case is the one that leaves a stale value in a real workflow (see the comment on ExecuteNonQuery.cs).

Suggestion: bind the output to a pre-seeded variable so the semantics is actually pinned down. Wrap the activity in a Sequence carrying a Variable<int> { Default = 42 }, bind AffectedRecords to it, and read it back afterwards; CaptureConnection further down this file is a ready made template for the read-back activity. Then assert 42 if stale values are intended, or 0 if the reset is.