Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP 1.1 Node Process Dying on JS errors #200

Open
AlanMacdonald opened this issue Sep 6, 2024 · 1 comment
Open

HTTP 1.1 Node Process Dying on JS errors #200

AlanMacdonald opened this issue Sep 6, 2024 · 1 comment

Comments

@AlanMacdonald
Copy link

AlanMacdonald commented Sep 6, 2024

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant