From b14aac4b8ed4d6608aba1133b51f058c43e8b00e Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Tue, 14 May 2024 21:17:20 +0000 Subject: [PATCH 1/6] feat(XHR): report HTTP errors to the `onerror` event handler --- python/pythonmonkey/builtin_modules/XMLHttpRequest.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 } From ebdef836c0e880585f63f9d564af79791d9e5452 Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Tue, 14 May 2024 21:37:49 +0000 Subject: [PATCH 2/6] test(XHR): fix error throwing Passing an object to `new Error` will result in `Error: [object Object]`. --- tests/js/xhr.simple | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) 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(); From 22a160580c6dfa356d39aa920aae2a79d8f56d21 Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Thu, 23 May 2024 17:27:02 +0000 Subject: [PATCH 3/6] feat(XHR): support HTTP-Keep-Alive --- python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py index 19c5a0d6..a7ae2e70 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 = aiohttp.TCPConnector(keepalive_timeout=15) # seconds before closing Keep-Alive connection + class XHRResponse(TypedDict, total=True): """ @@ -80,6 +82,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') From dffe510e25b17ce61f9fe72e4ce8df3231a4bde8 Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Thu, 30 May 2024 17:31:32 +0000 Subject: [PATCH 4/6] fix(XHR): `keepAliveConnector` attached to a different loop --- .../builtin_modules/XMLHttpRequest-internal.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py index a7ae2e70..af11208b 100644 --- a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py +++ b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py @@ -12,7 +12,7 @@ import pythonmonkey as pm from typing import Union, ByteString, Callable, TypedDict -keepAliveConnector = aiohttp.TCPConnector(keepalive_timeout=15) # seconds before closing Keep-Alive connection +keepAliveConnector: Union[aiohttp.TCPConnector, None] = None class XHRResponse(TypedDict, total=True): @@ -48,6 +48,11 @@ async def request( ): debug = pm.bootstrap.require("debug") + # to support HTTP-Keep-Alive + global keepAliveConnector + if (not keepAliveConnector): + keepAliveConnector = aiohttp.TCPConnector(keepalive_timeout=15) # seconds before closing Keep-Alive connection + class BytesPayloadWithProgress(aiohttp.BytesPayload): _chunkMaxLength = 2**16 # aiohttp default From f2f0e1e9f10603dc85049f276b81d804bd9c096e Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Thu, 30 May 2024 18:44:43 +0000 Subject: [PATCH 5/6] test(XHR): test HTTP-Keep-Alive automatically in the CI by tracing system calls that open a TCP connection --- tests/js/xhr-http-keep-alive.bash | 72 +++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tests/js/xhr-http-keep-alive.bash 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 $? From d495fddd7585bb70c334e627b0ad784467a4ab08 Mon Sep 17 00:00:00 2001 From: Tom Tang Date: Mon, 3 Jun 2024 19:36:23 +0000 Subject: [PATCH 6/6] feat(XHR): change the default HTTP-Keep-Alive to 5s 5s is the default for Node.js's `http.globalAgent`, see the change history in https://nodejs.org/dist/latest/docs/api/http.html#httpglobalagent --- python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py index af11208b..e3ea1824 100644 --- a/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py +++ b/python/pythonmonkey/builtin_modules/XMLHttpRequest-internal.py @@ -51,7 +51,8 @@ async def request( # to support HTTP-Keep-Alive global keepAliveConnector if (not keepAliveConnector): - keepAliveConnector = aiohttp.TCPConnector(keepalive_timeout=15) # seconds before closing Keep-Alive connection + # 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