Skip to content

Commit 427150a

Browse files
node24 fallback logic update (#5499)
* node24 fallback logic update * minor changes * fix and add test cases * remove duplicate code
1 parent f96983b commit 427150a

File tree

5 files changed

+180
-19
lines changed

5 files changed

+180
-19
lines changed

src/Agent.Worker/Handlers/NodeHandler.cs

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public interface INodeHandlerHelper
3232
string[] GetFilteredPossibleNodeFolders(string nodeFolderName, string[] possibleNodeFolders);
3333
string GetNodeFolderPath(string nodeFolderName, IHostContext hostContext);
3434
bool IsNodeFolderExist(string nodeFolderName, IHostContext hostContext);
35+
bool IsNodeExecutable(string nodeFolder, IHostContext HostContext, IExecutionContext ExecutionContext);
3536
}
3637

3738
public class NodeHandlerHelper : INodeHandlerHelper
@@ -52,6 +53,36 @@ public string[] GetFilteredPossibleNodeFolders(string nodeFolderName, string[] p
5253
possibleNodeFolders.Skip(nodeFolderIndex + 1).ToArray()
5354
: Array.Empty<string>();
5455
}
56+
57+
public bool IsNodeExecutable(string nodeFolder, IHostContext HostContext, IExecutionContext ExecutionContext)
58+
{
59+
if (!this.IsNodeFolderExist(nodeFolder, HostContext))
60+
{
61+
ExecutionContext.Debug($"Node folder does not exist: {nodeFolder}");
62+
return false;
63+
}
64+
var nodePath = Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Externals), nodeFolder, "bin", $"node{IOUtil.ExeExtension}");
65+
const int NodeNotExecutableExitCode = 216;
66+
try
67+
{
68+
var processInvoker = HostContext.CreateService<IProcessInvoker>();
69+
var exitCodeTask = processInvoker.ExecuteAsync(
70+
workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work),
71+
fileName: nodePath,
72+
arguments: "-v",
73+
environment: null,
74+
requireExitCodeZero: false,
75+
outputEncoding: null,
76+
cancellationToken: CancellationToken.None);
77+
int exitCode = exitCodeTask.GetAwaiter().GetResult();
78+
return exitCode != NodeNotExecutableExitCode;
79+
}
80+
catch (Exception ex)
81+
{
82+
ExecutionContext.Debug($"Node executable test threw exception: {ex.Message}");
83+
return false;
84+
}
85+
}
5586
}
5687

5788
public sealed class NodeHandler : Handler, INodeHandler
@@ -215,7 +246,7 @@ public async Task RunAsync()
215246
supportsNode20 = !node20ResultsInGlibCErrorHost;
216247
}
217248
}
218-
249+
219250
if (!useNode24InUnsupportedSystem)
220251
{
221252
if (supportsNode24.HasValue)
@@ -340,24 +371,24 @@ private string GetNodeFolderWithFallback(string preferredNodeFolder, bool node20
340371
switch (preferredNodeFolder)
341372
{
342373
case var folder when folder == NodeHandler.Node24Folder:
343-
// Fallback if Node24 has glibc error OR doesn't exist (e.g., win-x86)
344-
bool node24NotAvailable = !nodeHandlerHelper.IsNodeFolderExist(NodeHandler.Node24Folder, HostContext);
345-
if (node24ResultsInGlibCError || node24NotAvailable)
374+
// Fallback if Node24 has glibc error OR doesn't exist (e.g., win-x86) or not executable (e.g, windows 2012 R2)
375+
bool node24NotExecutable = !nodeHandlerHelper.IsNodeExecutable(NodeHandler.Node24Folder, this.HostContext, this.ExecutionContext);
376+
if (node24ResultsInGlibCError || node24NotExecutable)
346377
{
347378
// Fallback to Node20, then Node16 if Node20 also fails or doesn't exist
348379
bool node20NotAvailableForNode24Fallback = !nodeHandlerHelper.IsNodeFolderExist(NodeHandler.Node20_1Folder, HostContext);
349380
if (node20ResultsInGlibCError || node20NotAvailableForNode24Fallback)
350381
{
351-
fallbackReason = node24NotAvailable ? "NodeNotAvailable" : "GlibCError";
382+
fallbackReason = node24NotExecutable ? "NodeNotExecutable" : "GlibCError";
352383
fallbackOccurred = true;
353-
NodeFallbackWarning("24", "16", inContainer, node24NotAvailable);
384+
NodeFallbackWarning("24", "16", inContainer, node24NotExecutable);
354385
return NodeHandler.Node16Folder;
355386
}
356387
else
357388
{
358-
fallbackReason = node24NotAvailable ? "NodeNotAvailable" : "GlibCError";
389+
fallbackReason = node24NotExecutable ? "NodeNotExecutable" : "GlibCError";
359390
fallbackOccurred = true;
360-
NodeFallbackWarning("24", "20", inContainer, node24NotAvailable);
391+
NodeFallbackWarning("24", "20", inContainer, node24NotExecutable);
361392
return NodeHandler.Node20_1Folder;
362393
}
363394
}
@@ -383,15 +414,15 @@ private string GetNodeFolderWithFallback(string preferredNodeFolder, bool node20
383414
public string GetNodeLocation(bool node20ResultsInGlibCError, bool node24ResultsInGlibCError, bool inContainer)
384415
{
385416
bool useStrategyPattern = AgentKnobs.UseNodeVersionStrategy.GetValue(ExecutionContext).AsBoolean();
386-
417+
387418
if (useStrategyPattern)
388419
{
389420
return GetNodeLocationUsingStrategy(inContainer).GetAwaiter().GetResult();
390421
}
391-
422+
392423
return GetNodeLocationLegacy(node20ResultsInGlibCError, node24ResultsInGlibCError, inContainer);
393424
}
394-
425+
395426
private async Task<string> GetNodeLocationUsingStrategy(bool inContainer)
396427
{
397428
try
@@ -413,7 +444,7 @@ private async Task<string> GetNodeLocationUsingStrategy(bool inContainer)
413444
throw;
414445
}
415446
}
416-
447+
417448
private string GetNodeLocationLegacy(bool node20ResultsInGlibCError, bool node24ResultsInGlibCError, bool inContainer)
418449
{
419450
if (!string.IsNullOrEmpty(ExecutionContext.StepTarget()?.CustomNodePath))
@@ -539,11 +570,11 @@ private string GetNodeLocationLegacy(bool node20ResultsInGlibCError, bool node24
539570
return nodeHandlerHelper.GetNodeFolderPath(nodeFolder, HostContext);
540571
}
541572

542-
private void NodeFallbackWarning(string fromVersion, string toVersion, bool inContainer, bool notAvailable = false)
573+
private void NodeFallbackWarning(string fromVersion, string toVersion, bool inContainer, bool notExecutable = false)
543574
{
544575
string systemType = inContainer ? "container" : "agent";
545-
string reason = notAvailable
546-
? $"Node{fromVersion} is not available on this platform"
576+
string reason = notExecutable
577+
? $"Node{fromVersion} is not executable on this platform(e.g.,node binary missing or incompatible) "
547578
: $"The {systemType} operating system doesn't support Node{fromVersion}";
548579

549580
ExecutionContext.Warning($"{reason}. Using Node{toVersion} instead. " +

src/Agent.Worker/NodeVersionStrategies/Node24Strategy.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,13 @@ public NodeRunnerInfo CanHandle(TaskContext context, IExecutionContext execution
2828
bool useNode24Globally = AgentKnobs.UseNode24.GetValue(executionContext).AsBoolean();
2929
bool useNode24WithHandlerData = AgentKnobs.UseNode24withHandlerData.GetValue(executionContext).AsBoolean();
3030
bool eolPolicyEnabled = AgentKnobs.EnableEOLNodeVersionPolicy.GetValue(executionContext).AsBoolean();
31-
3231
var hostContext = executionContext.GetHostContext();
3332
string node24Folder = NodeVersionHelper.GetFolderName(NodeVersion.Node24);
3433
string taskName = executionContext.Variables.Get(Constants.Variables.Task.DisplayName) ?? "Unknown Task";
3534

36-
if (!_nodeHandlerHelper.IsNodeFolderExist(node24Folder, hostContext))
35+
if (!_nodeHandlerHelper.IsNodeExecutable(node24Folder, hostContext, executionContext))
3736
{
38-
executionContext.Debug("[Node24Strategy] Node24 binary not available on this platform, skipping");
37+
executionContext.Debug("[Node24Strategy] Node24 not executable on this platform (e.g., node binary missing or incompatible or exit code 216), checking fallback options");
3938
return null;
4039
}
4140

src/Test/L0/NodeHandlerL0.TestSpecifications.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,15 @@ public static class NodeHandlerTestSpecs
615615
expectedNode: "node24"
616616
),
617617

618+
new TestScenario(
619+
name: "Node24NotExecutable_fallsBackToNode20_1",
620+
description: "Node24 handler with Node24 not executable: falls back to Node20_1 in container",
621+
handlerData: typeof(Node20_1HandlerData),
622+
knobs: new() { ["AGENT_USE_NODE24_WITH_HANDLER_DATA"] = "true" },
623+
node24Executable: false,
624+
expectedNode: "node20_1"
625+
),
626+
618627
// ========================================================================================
619628
// GROUP 7: EDGE CASES AND ERROR SCENARIOS
620629
// ========================================================================================
@@ -816,7 +825,7 @@ public static class NodeHandlerTestSpecs
816825
},
817826
expectedNode: "node24",
818827
inContainer: true
819-
)
828+
)
820829
};
821830
}
822831

@@ -837,6 +846,7 @@ public class TestScenario
837846
public bool Node24GlibcError { get; set; }
838847
public bool InContainer { get; set; }
839848
public string CustomNodePath { get; set; }
849+
public bool Node24Executable { get; set; }
840850

841851
// Expected results (for equivalent scenarios)
842852
public string ExpectedNode { get; set; }
@@ -859,6 +869,7 @@ public TestScenario(
859869
Type expectedErrorType = null,
860870
bool node20GlibcError = false,
861871
bool node24GlibcError = false,
872+
bool node24Executable = true,
862873
bool inContainer = false,
863874
string customNodePath = null
864875
)
@@ -874,6 +885,7 @@ public TestScenario(
874885
ExpectedErrorType = expectedErrorType;
875886
Node20GlibcError = node20GlibcError;
876887
Node24GlibcError = node24GlibcError;
888+
Node24Executable = node24Executable;
877889
InContainer = inContainer;
878890
CustomNodePath = customNodePath;
879891
}

src/Test/L0/NodeHandlerL0.cs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
using Moq;
1313
using Xunit;
1414
using Agent.Sdk;
15+
using System.Threading;
16+
using System.Threading.Tasks;
17+
using System.Text;
1518

1619
namespace Microsoft.VisualStudio.Services.Agent.Tests
1720
{
@@ -86,6 +89,12 @@ public void UseNewNodeForNewNodeHandler(string nodeVersion)
8689
{
8790
thc.SetSingleton(new WorkerCommandManager() as IWorkerCommandManager);
8891
thc.SetSingleton(new ExtensionManager() as IExtensionManager);
92+
var processInvokerMock = new Mock<IProcessInvoker>();
93+
for (int i = 0; i < 10; i++)
94+
{
95+
thc.EnqueueInstance<IProcessInvoker>(processInvokerMock.Object);
96+
}
97+
SetupNodeProcessInvocation(processInvokerMock, nodeVersion, true);
8998

9099
NodeHandler nodeHandler = new NodeHandler(nodeHandlerHalper.Object);
91100

@@ -140,6 +149,12 @@ public void ForceUseNode24Knob(string nodeVersion)
140149
{
141150
thc.SetSingleton(new WorkerCommandManager() as IWorkerCommandManager);
142151
thc.SetSingleton(new ExtensionManager() as IExtensionManager);
152+
var processInvokerMock = new Mock<IProcessInvoker>();
153+
for (int i = 0; i < 10; i++)
154+
{
155+
thc.EnqueueInstance<IProcessInvoker>(processInvokerMock.Object);
156+
}
157+
SetupNodeProcessInvocation(processInvokerMock, nodeVersion, true);
143158

144159
NodeHandler nodeHandler = new NodeHandler(nodeHandlerHalper.Object);
145160

@@ -169,6 +184,64 @@ public void ForceUseNode24Knob(string nodeVersion)
169184
}
170185
}
171186

187+
[Theory]
188+
[InlineData("node24")]
189+
[Trait("Level", "L0")]
190+
[Trait("Category", "Common")]
191+
public void Node24NotExecutable(string nodeVersion)
192+
{
193+
ResetNodeKnobs();
194+
195+
Environment.SetEnvironmentVariable("AGENT_USE_NODE24_WITH_HANDLER_DATA", "true");
196+
197+
try
198+
{
199+
// Use a unique test name per data row to avoid sharing the same trace file across parallel runs
200+
using (TestHostContext thc = CreateTestHostContext($"{nameof(Node24NotExecutable)}_{nodeVersion}"))
201+
{
202+
thc.SetSingleton(new WorkerCommandManager() as IWorkerCommandManager);
203+
thc.SetSingleton(new ExtensionManager() as IExtensionManager);
204+
var processInvokerMock = new Mock<IProcessInvoker>();
205+
for (int i = 0; i < 10; i++)
206+
{
207+
thc.EnqueueInstance<IProcessInvoker>(processInvokerMock.Object);
208+
}
209+
SetupNodeProcessInvocation(processInvokerMock, nodeVersion, false);
210+
211+
nodeHandlerHalper
212+
.Setup(x => x.IsNodeExecutable(
213+
It.Is<string>(folder => folder == "node24"),
214+
It.IsAny<IHostContext>(),
215+
It.IsAny<IExecutionContext>()))
216+
.Returns(false);
217+
NodeHandler nodeHandler = new NodeHandler(nodeHandlerHalper.Object);
218+
219+
nodeHandler.Initialize(thc);
220+
nodeHandler.ExecutionContext = CreateTestExecutionContext(thc);
221+
nodeHandler.Data = nodeVersion switch
222+
{
223+
"node" => new NodeHandlerData(),
224+
"node10" => new Node10HandlerData(),
225+
"node16" => new Node16HandlerData(),
226+
"node20_1" => new Node20_1HandlerData(),
227+
"node24" => new Node24HandlerData(),
228+
_ => throw new Exception("Invalid node version"),
229+
};
230+
231+
string actualLocation = nodeHandler.GetNodeLocation(node20ResultsInGlibCError: false, node24ResultsInGlibCError: false, inContainer: false);
232+
string expectedLocation = Path.Combine(thc.GetDirectory(WellKnownDirectory.Externals),
233+
"node20_1",
234+
"bin",
235+
$"node{IOUtil.ExeExtension}");
236+
Assert.Equal(expectedLocation, actualLocation);
237+
}
238+
}
239+
finally
240+
{
241+
Environment.SetEnvironmentVariable("AGENT_USE_NODE24_WITH_HANDLER_DATA", null);
242+
}
243+
}
244+
172245
//tests that Node24 is NOT used when handler data exists but knob is false
173246
[Fact]
174247
[Trait("Level", "L0")]
@@ -574,9 +647,28 @@ private Mock<INodeHandlerHelper> GetMockedNodeHandlerHelper()
574647
.Setup(x => x.GetFilteredPossibleNodeFolders(It.IsAny<string>(), It.IsAny<string[]>()))
575648
.Returns(Array.Empty<string>);
576649

650+
nodeHandlerHelper
651+
.Setup(x => x.IsNodeExecutable(It.IsAny<string>(), It.IsAny<IHostContext>(), It.IsAny<IExecutionContext>()))
652+
.Returns(true);
653+
577654
return nodeHandlerHelper;
578655
}
579656

657+
private void SetupNodeProcessInvocation(Mock<IProcessInvoker> processInvokerMock, string nodeFolder, bool node24Executable)
658+
{
659+
string nodeExePath = Path.Combine("externals", nodeFolder, "bin", $"node{IOUtil.ExeExtension}");
660+
661+
processInvokerMock.Setup(x => x.ExecuteAsync(
662+
It.IsAny<string>(),
663+
It.Is<string>(fileName => fileName.Contains(nodeExePath)),
664+
"-v",
665+
It.IsAny<IDictionary<string, string>>(),
666+
false,
667+
It.IsAny<Encoding>(),
668+
It.IsAny<CancellationToken>()))
669+
.ReturnsAsync(node24Executable ? 0 : 216);
670+
}
671+
580672
private void ResetNodeKnobs()
581673
{
582674
Environment.SetEnvironmentVariable("AGENT_USE_NODE10", null);

src/Test/L0/NodeHandlerTestBase.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useStrategy)
7272
var dockerManagerMock = SetupMockedDockerCommandManager(scenario);
7373
thc.SetSingleton<IDockerCommandManager>(dockerManagerMock.Object);
7474

75+
// Mock IProcessInvoker for node executable checks (e.g., IsNodeExecutable in Node24Strategy)
76+
var processInvokerMock = new Mock<IProcessInvoker>();
77+
for (int i = 0; i < 10; i++)
78+
{
79+
thc.EnqueueInstance<IProcessInvoker>(processInvokerMock.Object);
80+
}
81+
82+
SetupNodeProcessInvocation(processInvokerMock, scenario.HandlerDataType.Name, scenario.Node24Executable);
83+
7584
var expectations = GetScenarioExpectations(scenario, useStrategy);
7685
try{
7786
string actualLocation;
@@ -242,6 +251,9 @@ private void ConfigureNodeHandlerHelper(TestScenario scenario)
242251
nodeFolderName,
243252
"bin",
244253
$"node{IOUtil.ExeExtension}"));
254+
NodeHandlerHelper
255+
.Setup(x => x.IsNodeExecutable(It.IsAny<string>(), It.IsAny<IHostContext>(), It.IsAny<IExecutionContext>()))
256+
.Returns(scenario.Node24Executable);
245257
}
246258

247259
private string GetExpectedNodeLocation(string expectedNode, TestScenario scenario, TestHostContext thc)
@@ -452,6 +464,21 @@ protected void ResetEnvironment()
452464
Environment.SetEnvironmentVariable("AGENT_USE_NODE24_IN_UNSUPPORTED_SYSTEM", null);
453465

454466
}
467+
468+
private void SetupNodeProcessInvocation(Mock<IProcessInvoker> processInvokerMock, string nodeFolder, bool node24Executable)
469+
{
470+
string nodeExePath = Path.Combine("externals", nodeFolder, "bin", $"node{IOUtil.ExeExtension}");
471+
472+
processInvokerMock.Setup(x => x.ExecuteAsync(
473+
It.IsAny<string>(),
474+
It.Is<string>(fileName => fileName.Contains(nodeExePath)),
475+
"-v",
476+
It.IsAny<IDictionary<string, string>>(),
477+
false,
478+
It.IsAny<Encoding>(),
479+
It.IsAny<CancellationToken>()))
480+
.ReturnsAsync(node24Executable ? 0 : 216);
481+
}
455482
}
456483

457484
public class TestResult

0 commit comments

Comments
 (0)