Skip to content

fix: prevent exceptions when listing indices and constraints asynchronously #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alexmg wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix: prevent exceptions when listing indices and constraints asynchronously #61

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
719a578
fix: ListIndicesAsync and ListConstraintsAsync methods throw exceptions
alexmg Mar 17, 2026
ce4ea65
fix: implement YIELD support in function calls
alexmg Mar 18, 2026
279a325
fix: extra null check extracting YIELD args
alexmg Mar 18, 2026
cc138fa
test: tiny write in tests to catch any graph init issues
alexmg Mar 18, 2026
ad85e3a
Merge branch 'master' into fix/async-index-helpers
alexmg Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions NFalkorDB.Tests/FalkorDBAPITest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Xunit;
using static NFalkorDB.Statistics;

Expand Down Expand Up @@ -415,6 +416,22 @@ public void TestIndexHelpersListIndices()
Assert.NotNull(indicesResult);
}

[Fact]
public async Task TestIndexHelpersListIndicesAsync()
{
var indicesResult = await _api.ListIndicesAsync();
// Shape is server-dependent; just ensure the call succeeds and returns a header
Assert.NotNull(indicesResult);
}

[Fact]
public async Task TestIndexHelpersListConstraintsAsync()
{
var constraints = await _api.ListConstraintsAsync();
// Shape is server-dependent; just ensure the call succeeds and returns a header
Assert.NotNull(constraints);
}

[Fact]
public void TestHeader()
{
Expand Down
56 changes: 18 additions & 38 deletions NFalkorDB/Graph.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,18 +233,9 @@ public async Task<ResultSet> ReadOnlyQueryAsync(string query, IDictionary<string
/// <returns>A result set.</returns>
public ResultSet CallProcedure(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
{
args = args?.Select(a => QuoteString(a));
var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);

var queryBody = new StringBuilder();

queryBody.Append(args != null ? $"CALL {procedure}({string.Join(",", args)})" : $"CALL {procedure}()");

if (kwargs != null && kwargs.TryGetValue("y", out var kwargsList))
{
queryBody.Append(string.Join(",", kwargsList));
}

return Query(queryBody.ToString(), flags: flags);
return Query(queryBody, flags: flags);
}

/// <summary>
Expand All @@ -255,20 +246,11 @@ public ResultSet CallProcedure(string procedure, IEnumerable<string> args = null
/// <param name="kwargs">A collection of keyword arguments.</param>
/// <param name="flags">[Optional] Command flags that are to be sent to the StackExchange.Redis connection multiplexer...</param>///
/// <returns>A result set.</returns>
public Task<ResultSet> CallProcedureAsync(string procedure, IEnumerable<string> args, Dictionary<string, List<string>> kwargs, CommandFlags flags = CommandFlags.None)
public Task<ResultSet> CallProcedureAsync(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
{
args = args.Select(a => QuoteString(a));

var queryBody = new StringBuilder();

queryBody.Append($"CALL {procedure}({string.Join(",", args)})");

if (kwargs.TryGetValue("y", out var kwargsList))
{
queryBody.Append(string.Join(",", kwargsList));
}
var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);

return QueryAsync(queryBody.ToString(), flags: flags);
return QueryAsync(queryBody, flags: flags);
}
Comment on lines +249 to 254
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve the previous CallProcedureAsync overload.

This public signature now puts flags after two optional parameters. Existing consumers that called CallProcedureAsync("db.labels", CommandFlags.None, ...) will break. Keep the old overload and forward it to this implementation instead of replacing it.

Compatibility overload 
+    public Task<ResultSet> CallProcedureAsync(
+        string procedure,
+        CommandFlags flags,
+        IEnumerable<string> args = null,
+        Dictionary<string, List<string>> kwargs = null) =>
+        CallProcedureAsync(procedure, args, kwargs, flags);
+
     public Task<ResultSet> CallProcedureAsync(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
     {
         var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);
 
         return QueryAsync(queryBody, flags: flags);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Task<ResultSet> CallProcedureAsync(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
{
args = args.Select(a => QuoteString(a));
var queryBody = new StringBuilder();
queryBody.Append($"CALL {procedure}({string.Join(",", args)})");
if (kwargs.TryGetValue("y", out var kwargsList))
{
queryBody.Append(string.Join(",", kwargsList));
}
var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);
return QueryAsync(queryBody.ToString(), flags: flags);
return QueryAsync(queryBody, flags: flags);
}
public Task<ResultSet> CallProcedureAsync(
string procedure,
CommandFlags flags,
IEnumerable<string> args = null,
Dictionary<string, List<string>> kwargs = null) =>
CallProcedureAsync(procedure, args, kwargs, flags);
public Task<ResultSet> CallProcedureAsync(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
{
var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);
return QueryAsync(queryBody, flags: flags);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@NFalkorDB/Graph.cs` around lines 249 - 254, The new CallProcedureAsync public
signature changed parameter order and breaks existing callers; restore the
previous overload that accepted (string procedure, CommandFlags flags,
IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null)
and implement it to forward into the current implementation (the overload that
calls BuildQueryBodyForProcedureCall and then QueryAsync) so old call sites like
CallProcedureAsync("db.labels", CommandFlags.None, ...) continue to work; keep
the existing implementation (which builds the query via
BuildQueryBodyForProcedureCall and calls QueryAsync) unchanged and simply add
the compatibility overload that delegates to it.

Copy link
Copy Markdown
Author

@alexmg alexmg Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The original CallProcedureAsync signature had args and kwargs parameters after the procedure parameter. Both parameters were non-nullable which was inconsistent with the other call procedure overloads that assign a default null value.

public Task<ResultSet> CallProcedureAsync(
    string procedure,
    IEnumerable<string> args,
    Dictionary<string, List<string>> kwargs,
    CommandFlags flags = CommandFlags.None)

Invoking the CallProcedureAsync method with null values resulted in a NullReferenceException being thrown.

CallProcedureAsync("DB.CONSTRAINTS", null, null, flags);

The cause being the args.Select() statement on line 1 attempting to enumerate a null instance.

public Task<ResultSet> CallProcedureAsync(
    string procedure,
    IEnumerable<string> args,
    Dictionary<string, List<string>> kwargs,
    CommandFlags flags = CommandFlags.None)
{
    args = args.Select(a => QuoteString(a)); // NullReferenceException!

    // ...
}

It was mandatory to provide non-null values for those parameters when invoking the previous signature.

CallProcedureAsync("foo", [], [], CommandFlags.None);

The method CallProcedureAsync("db.labels", CommandFlags.None, ...) was not a valid signature prior to the commit because of the non-nullable args and kwargs parameters.


/// <summary>
Expand All @@ -281,18 +263,9 @@ public Task<ResultSet> CallProcedureAsync(string procedure, IEnumerable<string>
/// <returns>A result set.</returns>
public ResultSet CallProcedureReadOnly(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
{
args = args.Select(a => QuoteString(a));

var queryBody = new StringBuilder();

queryBody.Append($"CALL {procedure}({string.Join(",", args)})");
var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);

if (kwargs != null && kwargs.TryGetValue("y", out var kwargsList))
{
queryBody.Append(string.Join(",", kwargsList));
}

return ReadOnlyQuery(queryBody.ToString(), flags: flags);
return ReadOnlyQuery(queryBody, flags: flags);
}

/// <summary>
Expand All @@ -305,18 +278,25 @@ public ResultSet CallProcedureReadOnly(string procedure, IEnumerable<string> arg
/// <returns>A result set.</returns>
public Task<ResultSet> CallProcedureReadOnlyAsync(string procedure, IEnumerable<string> args = null, Dictionary<string, List<string>> kwargs = null, CommandFlags flags = CommandFlags.None)
{
args = args.Select(a => QuoteString(a));
var queryBody = BuildQueryBodyForProcedureCall(procedure, ref args, kwargs);

return ReadOnlyQueryAsync(queryBody, flags: flags);
}

private static string BuildQueryBodyForProcedureCall(string procedure, ref IEnumerable<string> args, Dictionary<string, List<string>> kwargs)
{
args = args?.Select(QuoteString);

var queryBody = new StringBuilder();

queryBody.Append($"CALL {procedure}({string.Join(",", args)})");
queryBody.Append(args != null ? $"CALL {procedure}({string.Join(",", args)})" : $"CALL {procedure}()");
Comment on lines +288 to +292
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Guard against null items inside args to avoid runtime NREs.

Line 288 calls QuoteString over each item, and QuoteString dereferences input; a null element in args will currently crash with NullReferenceException instead of a clear argument error.

Suggested fix 
 internal static string BuildQueryBodyForProcedureCall(string procedure, ref IEnumerable<string> args, Dictionary<string, List<string>> kwargs)
 {
-    args = args?.Select(QuoteString);
+    var quotedArgs = args?
+        .Select(a => a ?? throw new System.ArgumentException("Procedure args cannot contain null values.", nameof(args)))
+        .Select(QuoteString)
+        .ToArray();
+
+    args = quotedArgs;
 
     var queryBody = new StringBuilder();
 
-    queryBody.Append(args != null ? $"CALL {procedure}({string.Join(",", args)})" : $"CALL {procedure}()");
+    queryBody.Append(quotedArgs != null ? $"CALL {procedure}({string.Join(",", quotedArgs)})" : $"CALL {procedure}()");
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@NFalkorDB/Graph.cs` around lines 288 - 292, The code maps args through
QuoteString which dereferences its input, so a null element in args will cause a
NullReferenceException; update the code around args = args?.Select(QuoteString)
to first validate that args does not contain any null items (e.g., check args !=
null && args.Any(a => a == null)) and throw a clear ArgumentException (including
the procedure name and parameter name "args") if a null is found, or
alternatively replace null elements with a safe representation before calling
QuoteString; keep references to args, QuoteString, queryBody and procedure so
the validation is applied just before building the CALL string.


if (kwargs.TryGetValue("y", out var kwargsList))
if (kwargs != null && kwargs.TryGetValue("y", out var kwargsList))
{
queryBody.Append(string.Join(",", kwargsList));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return ReadOnlyQueryAsync(queryBody.ToString(), flags: flags);
return queryBody.ToString();
}

/// <summary>
Expand Down