From de58204bbe29ba4e54ea940437e6523f1c552edc Mon Sep 17 00:00:00 2001 From: blushingpenguin Date: Tue, 23 Jun 2020 13:18:51 +0100 Subject: [PATCH 1/3] capture the final output from node processes to help diagnose unexpected node exits --- .../OutOfProcess/NodeJSProcess.cs | 32 +++++++++++-------- test/NodeJS/NodeJSProcessUnitTests.cs | 28 ++++++++++++++-- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/NodeJSProcess.cs b/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/NodeJSProcess.cs index fa2c660..c778ac7 100644 --- a/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/NodeJSProcess.cs +++ b/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; diff --git a/test/NodeJS/NodeJSProcessUnitTests.cs b/test/NodeJS/NodeJSProcessUnitTests.cs index a71c2a5..a54bea3 100644 --- a/test/NodeJS/NodeJSProcessUnitTests.cs +++ b/test/NodeJS/NodeJSProcessUnitTests.cs @@ -240,18 +240,40 @@ public void ExitStatus_ReturnsExitCodeIfProcessHasExitedButNotBeenDisposedOf() } [Fact] - public void DataReceivedHandler_DoesNothingIfEventDataIsNull() + public void DataReceivedHandler_DoesNothingIfEventDataIsNullAndThereIsNoBufferedData() { // Arrange Mock 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(), It.IsAny(), out dummyOutMessage), Times.Never()); + mockTestSubject.Verify(t => t.TryCreateMessage(It.IsAny(), It.IsAny(), out dummyOutMessage), Times.Once()); + Assert.Equal("__NOTCALLED__", capturedMessage); + } + + [Fact] + public void DataReceivedHandler_SendsBufferedDataIfEventDataIsNull() + { + // Arrange + Mock 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(), It.IsAny(), out dummyOutMessage), Times.Once()); + Assert.Equal("test message", capturedMessage); } [Fact] From 95df46a087cf47ba85cca59d82c43164db8c77aa Mon Sep 17 00:00:00 2001 From: JeremyTCD Date: Tue, 23 Jun 2020 23:42:56 +0800 Subject: [PATCH 2/3] Added release 5.4.1. --- Changelog.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 04a1f5d..fbad1ba 100644 --- a/Changelog.md +++ b/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 From 2636ece685d6457b527a551c31cae21d3905e20d Mon Sep 17 00:00:00 2001 From: blushingpenguin Date: Tue, 23 Jun 2020 17:34:05 +0100 Subject: [PATCH 3/3] add blushingpenguin to contributors --- ReadMe.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ReadMe.md b/ReadMe.md index 159578f..3c79081 100644 --- a/ReadMe.md +++ b/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.