capture the final output from node processes to help diagnose unexpected node exits

Changelog.md

@@ -3,7 +3,11 @@ This project uses [semantic versioning](http://semver.org/spec/v2.0.0.html). Ref
 *[Semantic Versioning in Practice](https://www.jering.tech/articles/semantic-versioning-in-practice)*
 for an overview of semantic versioning.
 
-## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.0...HEAD)
+## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.1...HEAD)
+
+## [5.4.1](https://github.com/JeringTech/Javascript.NodeJS/compare/5.4.0...5.4.1) - Jun 23, 2020
+### Fixes
+- Fixed capturing of final output from Node.js. ([#84](https://github.com/JeringTech/Javascript.NodeJS/pull/84)).
 
 ## [5.4.0](https://github.com/JeringTech/Javascript.NodeJS/compare/5.3.2...5.4.0) - Mar 9, 2020
 ### Additions

ReadMe.md

@@ -1029,6 +1029,7 @@ Contributions are welcome!
 - [JeremyTCD](https://github.com/JeremyTCD)
 - [Daniil Sokolyuk](https://github.com/DaniilSokolyuk)
 - [dustinsoftware](https://github.com/dustinsoftware)
+- [blushingpenguin](https://github.com/blushingpenguin)
 
 ## About
 Follow [@JeringTech](https://twitter.com/JeringTech) for updates and more.

src/NodeJS/NodeJSServiceImplementations/OutOfProcess/NodeJSProcess.cs

@@ -207,10 +207,6 @@ internal virtual void DataReceivedHandler(StringBuilder stringBuilder,
             DataReceivedEventArgs dataReceivedEventArgs)
         {
             string data = dataReceivedEventArgs.Data;
-            if (data == null) // When the stream is closed, an event with data = null is received - https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.datareceivedeventargs.data?view=netframework-4.8#remarks
-            {
-                return;
-            }
 
             // Process output is received line by line. The last line of a message ends with a \0 (null character),
             // so we accumulate lines in a StringBuilder till the \0, then log the entire message in one go.
@@ -226,22 +222,32 @@ internal virtual void DataReceivedHandler(StringBuilder stringBuilder,
         internal virtual bool TryCreateMessage(StringBuilder stringBuilder, string data, out string message)
         {
             message = null;
-            int dataLength = data.Length;
 
-            if (dataLength == 0) // Empty line
+            // When the stream is closed, an event with data = null is received - https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.datareceivedeventargs.data?view=netframework-4.8#remarks
+            if (data != null)
             {
-                stringBuilder.AppendLine();
-                return false;
-            }
+                int dataLength = data.Length;
+
+                if (dataLength == 0) // Empty line
+                {
+                    stringBuilder.AppendLine();
+                    return false;
+                }
+
+                if (data[dataLength - 1] != '\0')
+                {
+                    stringBuilder.AppendLine(data);
+                    return false;
+                }
 
-            if (data[dataLength - 1] != '\0')
+                stringBuilder.Append(data);
+                stringBuilder.Length--; // Remove null terminating character
+            }
+            else if (stringBuilder.Length == 0)
             {
-                stringBuilder.AppendLine(data);
                 return false;
             }
 
-            stringBuilder.Append(data);
-            stringBuilder.Length--; // Remove null terminating character
             message = stringBuilder.ToString();
             stringBuilder.Length = 0;
 

test/NodeJS/NodeJSProcessUnitTests.cs

@@ -240,18 +240,40 @@ public void ExitStatus_ReturnsExitCodeIfProcessHasExitedButNotBeenDisposedOf()
         }
 
         [Fact]
-        public void DataReceivedHandler_DoesNothingIfEventDataIsNull()
+        public void DataReceivedHandler_DoesNothingIfEventDataIsNullAndThereIsNoBufferedData()
         {
             // Arrange
             Mock<NodeJSProcess> mockTestSubject = CreateMockNodeJSProcess();
             mockTestSubject.CallBase = true;
 
             // Act
-            mockTestSubject.Object.DataReceivedHandler(null, null, null, CreateDataReceivedEventArgs());
+            var sb = new StringBuilder();
+            string capturedMessage = "__NOTCALLED__";
+            mockTestSubject.Object.DataReceivedHandler(sb, (object sender, string message) => capturedMessage = message, null, CreateDataReceivedEventArgs());
 
             // Assert
             string dummyOutMessage = null;
-            mockTestSubject.Verify(t => t.TryCreateMessage(It.IsAny<StringBuilder>(), It.IsAny<string>(), out dummyOutMessage), Times.Never());
+            mockTestSubject.Verify(t => t.TryCreateMessage(It.IsAny<StringBuilder>(), It.IsAny<string>(), out dummyOutMessage), Times.Once());
+            Assert.Equal("__NOTCALLED__", capturedMessage);
+        }
+
+        [Fact]
+        public void DataReceivedHandler_SendsBufferedDataIfEventDataIsNull()
+        {
+            // Arrange
+            Mock<NodeJSProcess> mockTestSubject = CreateMockNodeJSProcess();
+            mockTestSubject.CallBase = true;
+
+            // Act
+            var sb = new StringBuilder();
+            sb.Append("test message");
+            string capturedMessage = null;
+            mockTestSubject.Object.DataReceivedHandler(sb, (object sender, string message) => capturedMessage = message, null, CreateDataReceivedEventArgs());
+
+            // Assert
+            string dummyOutMessage = null;
+            mockTestSubject.Verify(t => t.TryCreateMessage(It.IsAny<StringBuilder>(), It.IsAny<string>(), out dummyOutMessage), Times.Once());
+            Assert.Equal("test message", capturedMessage);
         }
 
         [Fact]