In version 6.X, state is lost after an exception is thrown

[Taritsyn]

Hello, Jeremy!

After upgrading from version 5.4.4 to 6.1.0, some my tests began to fail. Upon a detailed research, it turned out that after any errors occur in JavaScript code, state is lost.

To reproduce this error, I wrote a console application:

using System;
using System.Threading.Tasks;

using Jering.Javascript.NodeJS;

namespace TestJeringNodeJS
{
    public class Program
    {
        const string MODULE_FILE_NAME = "counter.js";
        const string MODULE_CODE = @"
let counter = 0;

module.exports.increaseCounter = (callback, num) => {
    if (num < 0) {
        throw new Error('The value cannot be negative!');
    }

    let result = counter += num;
    callback(null, result);
}";


        public static void Main(string[] args)
        {
            IncreaseCounter(3);
            IncreaseCounter(4);
            IncreaseCounter(-1);
            IncreaseCounter(5);
        }

        private static void IncreaseCounter(int num)
        {
            try
            {
                int result = InvokeEngineHelper<int>("increaseCounter", new object[] { num });

                Console.WriteLine("Result: {0}", result);
            }
            catch (InvocationException e)
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine(e.Message);
                Console.ForegroundColor = ConsoleColor.White;
            }
        }

        private static T InvokeEngineHelper<T>(string exportName = null, object[] args = null)
        {
            Task<(bool, T)> cachedTask = StaticNodeJSService.TryInvokeFromCacheAsync<T>(MODULE_FILE_NAME,
                exportName, args);
            (bool success, T result) = cachedTask.ConfigureAwait(false).GetAwaiter().GetResult();

            if (success)
            {
                return result;
            }
            else
            {
                Task<T> task = StaticNodeJSService.InvokeFromStringAsync<T>(MODULE_CODE, MODULE_FILE_NAME,
                    exportName, args);

                return task.ConfigureAwait(false).GetAwaiter().GetResult();
            }
        }
    }
}

When using version 5.4.4, state after the error occurs is preserved:

Result: 3
Result: 7
The value cannot be negative!
Error: The value cannot be negative!
    at module.exports.increaseCounter (anonymous:6:15)
    at Server.<anonymous> ([eval]:1:4360)
    at Generator.next (<anonymous>)
    at [eval]:1:1271
    at new Promise (<anonymous>)
    at r ([eval]:1:1016)
    at IncomingMessage.<anonymous> ([eval]:1:2634)
    at IncomingMessage.emit (events.js:327:22)
    at endReadableNT (internal/streams/readable.js:1327:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
Result: 12

When using version 6.1.0, state after the error occurs is lost:

Result: 3
Result: 7
The value cannot be negative!
Error: The value cannot be negative!
    at module.exports.increaseCounter (anonymous:6:15)
    at Server.<anonymous> ([eval]:1:2443)
    at Generator.next (<anonymous>)
    at [eval]:1:355
    at new Promise (<anonymous>)
    at o ([eval]:1:100)
    at IncomingMessage.<anonymous> ([eval]:1:557)
    at IncomingMessage.emit (events.js:327:22)
    at endReadableNT (internal/streams/readable.js:1327:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
Result: 5

[JeremyTCD]

Hi Taritsyn!

6.1.0 includes a new feature, process retries:

OutOfProcessNodeJSServiceOptions.NumProcessRetries

The number of new NodeJS processes created to retry an invocation.

A NodeJS process retries invocations OutOfProcessNodeJSServiceOptions.NumRetries times. Once a process's retries are exhausted, if any process retries remain, the library creates a new process that then retries invocations OutOfProcessNodeJSServiceOptions.NumRetries times.

For example, consider the situation where OutOfProcessNodeJSServiceOptions.NumRetries and this value are both 1. The existing process first attempts the invocation. If it fails, it retries the invocation once. If it fails again, the library creates a new process that retries the invocation once. In total, the library attempt the invocation 3 times.

This was added to deal with "established connection was aborted" issues. In these issues the .Net process stops being able to communicate with the existing Node.js process.

The new feature works around these issues by retrying in a new process, hence the loss of state.

One solution would be to set OutOfProcessNodeJSServiceOptions.NumProcessRetries to 0. This would disable the new feature. Could you try that and see if the issue goes away?

Disabling the feature would leave applications vulnerable to "aborted connection" issues though.

I think I'll change the default behavior of process retries so they only occur on connection issues, not on JS errors. This would deal with "aborted" issues without messing up state on JS errors. Will add a flag for users who want process retries on JS errors too. Let me know if you have any suggestions!

[Taritsyn]

Hello, Jeremy!

One solution would be to set OutOfProcessNodeJSServiceOptions.NumProcessRetries to 0. This would disable the new feature. Could you try that and see if the issue goes away?

Yes, this setting solved my problem.

I think I'll change the default behavior of process retries so they only occur on connection issues, not on JS errors. This would deal with "aborted" issues without messing up state on JS errors.

This would be perfect solution.

Will add a flag for users who want process retries on JS errors too. Let me know if you have any suggestions!

In my case, it would be nice if this flag was set to false by default.

[JeremyTCD]

I've released 6.2.0 with the flag. Defaults to false. Let me know if you have any other issues :).

[Taritsyn]

Thank you very much!