diff --git a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py index 19c5a0d6..e3ea1824 100644 --- a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py +++ b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py @@ -12,6 +12,8 @@ import pythonmonkey as pm from typing import Union, ByteString, Callable, TypedDict +keepAliveConnector: Union[aiohttp.TCPConnector, None] = None + class XHRResponse(TypedDict, total=True): """ @@ -46,6 +48,12 @@ async def request( ): debug = pm.bootstrap.require("debug") + # to support HTTP-Keep-Alive + global keepAliveConnector + if (not keepAliveConnector): + # seconds before closing Keep-Alive connection. + keepAliveConnector = aiohttp.TCPConnector(keepalive_timeout=5) # 5s is the default for Node.js's `http.globalAgent` + class BytesPayloadWithProgress(aiohttp.BytesPayload): _chunkMaxLength = 2**16 # aiohttp default @@ -80,6 +88,7 @@ async def write(self, writer) -> None: headers=headers, data=BytesPayloadWithProgress(body) if body else None, timeout=timeoutOptions, + connector=keepAliveConnector, ) as res: debug('xhr:aiohttp')('got', res.content_type, 'result') diff --git a/python/pythonmonkey/builtin_modules/XMLHttpRequest.js b/python/pythonmonkey/builtin_modules/XMLHttpRequest.js index 82dc4a99..6db9bc04 100644 --- a/python/pythonmonkey/builtin_modules/XMLHttpRequest.js +++ b/python/pythonmonkey/builtin_modules/XMLHttpRequest.js @@ -49,7 +49,7 @@ class ProgressEvent extends Event { /** * @param {string} type - * @param {{ lengthComputable?: boolean; loaded?: number; total?: number; }} eventInitDict + * @param {{ lengthComputable?: boolean; loaded?: number; total?: number; error?: Error; }} eventInitDict */ constructor (type, eventInitDict = {}) { @@ -57,6 +57,7 @@ class ProgressEvent extends Event this.lengthComputable = eventInitDict.lengthComputable ?? false; this.loaded = eventInitDict.loaded ?? 0; this.total = eventInitDict.total ?? 0; + this.error = eventInitDict.error ?? null; this.debugTag = 'xhr:'; } } @@ -446,12 +447,12 @@ class XMLHttpRequest extends XMLHttpRequestEventTarget this.#uploadCompleteFlag = true; if (this.#uploadListenerFlag) { - this.#uploadObject.dispatchEvent(new ProgressEvent(event, { loaded:0, total:0 })); + this.#uploadObject.dispatchEvent(new ProgressEvent(event, { loaded:0, total:0, error: exception })); this.#uploadObject.dispatchEvent(new ProgressEvent('loadend', { loaded:0, total:0 })); } } - this.dispatchEvent(new ProgressEvent(event, { loaded:0, total:0 })); // step 7 + this.dispatchEvent(new ProgressEvent(event, { loaded:0, total:0, error: exception })); // step 7 this.dispatchEvent(new ProgressEvent('loadend', { loaded:0, total:0 })); // step 8 } diff --git a/tests/js/xhr-http-keep-alive.bash b/tests/js/xhr-http-keep-alive.bash new file mode 100644 index 00000000..3ac6a421 --- /dev/null +++ b/tests/js/xhr-http-keep-alive.bash @@ -0,0 +1,72 @@ +#! /bin/bash +# +# @file xhr-http-keep-alive.bash +# For testing HTTP-Keep-Alive automatically in the CI. +# +# We use `strace` to track system calls within the process that open a TCP connection. +# If HTTP-Keep-Alive is working, the total number of TCP connections opened should be 1 for a single remote host. +# +# @author Tom Tang (xmader@distributive.network) +# @date May 2024 + +set -u +set -o pipefail + +panic() +{ + echo "FAIL: $*" >&2 + exit 2 +} + +cd `dirname "$0"` || panic "could not change to test directory" + +if [[ "$OSTYPE" != "linux-gnu"* ]]; then + exit 0 + # Skip non-Linux for this test + # TODO: add tests on Windows and macOS. What's the equivalence of `strace`? +fi + +code=' +function newRequest(url) { + return new Promise(function (resolve, reject) + { + let xhr = new XMLHttpRequest(); + xhr.open("GET", url); + xhr.onload = function () + { + if (this.status >= 200 && this.status < 300) resolve(this.response); + else reject(new Error(this.status)); + }; + xhr.onerror = (ev) => reject(ev.error); + xhr.send(); + }); +} + +async function main() { + await newRequest("http://www.example.org/"); + await newRequest("http://www.example.org/"); + await newRequest("http://http.badssl.com/"); +} + +main(); +' + +# Trace the `socket` system call https://man.archlinux.org/man/socket.2 +# AF_INET: IPv4, IPPROTO_TCP: TCP connection +TRACE=$(strace -f -e socket \ + "${PMJS:-pmjs}" -e "$code" \ + < /dev/null 2>&1 +) + +# We have 3 HTTP requests, but we should only have 2 TCP connections open, +# as HTTP-Keep-Alive reuses the socket for a single remote host. +echo "$TRACE" \ +| tr -d '\r' \ +| grep -c -E 'socket\(AF_INET, \w*(\|\w*)+, IPPROTO_TCP\)' \ +| while read qty + do + echo "$TRACE" + echo "TCP connections opened: $qty" + [ "$qty" != "2" ] && panic qty should not be $qty + break + done || exit $? diff --git a/tests/js/xhr.simple b/tests/js/xhr.simple index 2afddb55..8c61e6dc 100644 --- a/tests/js/xhr.simple +++ b/tests/js/xhr.simple @@ -69,19 +69,16 @@ new Promise(function (resolve, reject) } else { - reject(new Error({ + reject(new Error(JSON.stringify({ status: this.status, - statusText: this.statusText - })); + statusText: this.statusText + }))); } }; - xhr.onerror = function () + xhr.onerror = function (ev) { - reject(new Error({ - status: this.status, - statusText: this.statusText - })); + reject(ev.error); }; xhr.send(); @@ -112,19 +109,16 @@ new Promise(function (resolve, reject) } else { - reject(new Error({ + reject(new Error(JSON.stringify({ status: this.status, - statusText: this.statusText - })); + statusText: this.statusText + }))); } }; - xhr.onerror = function () + xhr.onerror = function (ev) { - reject(new Error({ - status: this.status, - statusText: this.statusText - })); + reject(ev.error); }; xhr.send();