JeringTech / Javascript.NodeJS

Invoke Javascript in NodeJS, from C#
Other
463 stars 43 forks source link

HTTP 1.1 Node Process Dying on JS errors #200

Open AlanMacdonald opened 2 months ago

AlanMacdonald commented 2 months ago

Hello we are experiencing a problem with version 7, where when a javascript error occurs the node process dies, despite the fact we have a try catch and call the error callback with the error details. I debugged and see node throwing later in the request with an error relating to writing to the response that had already ended. I inspected the source code and I am almost certain on line 164 of Http11Server.ts there should be a return statement meaning the code will not then continue on to attempt to end the response a second time later (on line 175 in our case). respondWithError function already ends the response so it makes sense to return.

if (error != null) {
   respondWithError(res, error);
   return; //Proposed new line
}

Furthermore the equivalent line is already present in Http20Server.ts line 150.

Having found this I was going to attempt to verify the problem is indeed not present with HTTP 2 by setting the HTTP options, but then found the options object has a pragma disallowing me doing this for .net 4.8 so can't.

    public class HttpNodeJSServiceOptions
    {
#if NETCOREAPP3_1 || NET5_0_OR_GREATER
        /// <summary>The HTTP version to use.</summary>
        /// <remarks>
        /// <para>This value can be <see cref="HttpVersion.Version11"/> or <see cref="HttpVersion.Version20"/>. <see cref="HttpVersion.Version11"/> is faster than <see cref="HttpVersion.Version20"/>, 
        /// but <see cref="HttpVersion.Version20"/> may be more stable (unverified).</para>
        /// <para>If this value is not <see cref="HttpVersion.Version11"/> or <see cref="HttpVersion.Version20"/>, <see cref="HttpVersion.Version11"/> is used.</para>
        /// <para>This option is not available for the net461 and netstandard2.0 versions of this library because those framework versions do not support HTTP/2.0.</para>
        /// <para>Defaults to <see cref="HttpVersion.Version11"/>.</para>
        /// </remarks>
        public Version Version { get; set; } = HttpVersion.Version11;
#endif
    }
AlanMacdonald commented 1 month ago

I've verified as expected the process does not get killed on a JS error when using HTTP version 2.0 since it already has the return statement.