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

Fix Socket.IO connection timeout #236

Merged
merged 9 commits into from
Feb 16, 2024
47 changes: 42 additions & 5 deletions python/pythonmonkey/builtin_modules/XMLHttpRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,27 @@ class XMLHttpRequest extends XMLHttpRequestEventTarget
*/
timeout = 0;

/**
* A boolean value that indicates whether or not cross-site `Access-Control` requests should be made using credentials such as cookies, authorization headers or TLS client certificates.
* Setting withCredentials has no effect on same-origin requests.
* @see https://xhr.spec.whatwg.org/#the-withcredentials-attribute
*/
get withCredentials()
{
return false;
return this.#crossOriginCredentials;
}
set withCredentials(flag)
{
throw new DOMException('xhr.withCredentials is not supported in PythonMonkey.', 'InvalidAccessError');
// step 1
if (this.#state !== XMLHttpRequest.UNSENT && this.#state !== XMLHttpRequest.OPENED)
// The XHR internal state should be UNSENT or OPENED.
throw new DOMException('XMLHttpRequest must not be sending.', 'InvalidStateError');
// step 2
if (this.#sendFlag)
throw new DOMException('send() has already been called', 'InvalidStateError');
// step 3
this.#crossOriginCredentials = flag;
// TODO: figure out what cross-origin means in PythonMonkey. Is it always same-origin request? What to send?
}

/**
Expand Down Expand Up @@ -501,8 +515,8 @@ class XMLHttpRequest extends XMLHttpRequestEventTarget
{
if (this.#state === XMLHttpRequest.LOADING || this.#state === XMLHttpRequest.DONE)
throw new DOMException('responseType can only be set before send()', 'InvalidStateError');
if (!['', 'text', 'arraybuffer'].includes(t))
throw new DOMException('only responseType "text" or "arraybuffer" is supported', 'NotSupportedError');
if (!['', 'text', 'arraybuffer', 'json'].includes(t))
throw new DOMException('only responseType "text" or "arraybuffer" or "json" is supported', 'NotSupportedError');
this.#responseType = t;
}

Expand Down Expand Up @@ -536,7 +550,29 @@ class XMLHttpRequest extends XMLHttpRequestEventTarget
this.#responseObject = this.#mergeReceivedBytes().buffer;
return this.#responseObject;
}

if (this.#responseType === 'json') // step 8
{
// step 8.2
if (this.#receivedLength === 0) // response’s body is null
return null;
// step 8.3
let jsonObject = null;
try
{
// TODO: use proper TextDecoder API
const str = decodeStr(this.#mergeReceivedBytes(), 'utf-8'); // only supports utf-8, see https://infra.spec.whatwg.org/#parse-json-bytes-to-a-javascript-value
jsonObject = JSON.parse(str);
}
catch (exception)
{
return null;
}
// step 8.4
this.#responseObject = jsonObject;
}

// step 6 and step 7 ("blob" or "document") are not supported
throw new DOMException(`unsupported responseType "${this.#responseType}"`, 'InvalidStateError');
}

Expand Down Expand Up @@ -565,6 +601,7 @@ class XMLHttpRequest extends XMLHttpRequestEventTarget
#uploadObject = new XMLHttpRequestUpload();
#state = XMLHttpRequest.UNSENT; // One of unsent, opened, headers received, loading, and done; initially unsent.
#sendFlag = false; // A flag, initially unset.
#crossOriginCredentials = false; // A boolean, initially false.
/** @type {Method} */
#requestMethod = null;
/** @type {URL} */
Expand All @@ -585,7 +622,7 @@ class XMLHttpRequest extends XMLHttpRequestEventTarget
#responseType = '';
/**
* cache for converting receivedBytes to the desired response type
* @type {ArrayBuffer | string}
* @type {ArrayBuffer | string | Record<any, any>}
*/
#responseObject = null;

Expand Down
4 changes: 4 additions & 0 deletions python/pythonmonkey/builtin_modules/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ function setTimeout(handler, delayMs = 0, ...args)
*/
function clearTimeout(timeoutId)
{
// silently does nothing when an invalid timeoutId (should be an int32 value) is passed in
if (!Number.isInteger(timeoutId))
Xmader marked this conversation as resolved.
Show resolved Hide resolved
return;

return cancelByTimeoutId(timeoutId);
}

Expand Down
11 changes: 2 additions & 9 deletions src/internalBinding/timers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,18 @@ static bool enqueueWithDelay(JSContext *cx, unsigned argc, JS::Value *vp) {
PyEventLoop::AsyncHandle handle = loop.enqueueWithDelay(job, delaySeconds);

// Return the `timeoutID` to use in `clearTimeout`
args.rval().setDouble((double)PyEventLoop::AsyncHandle::getUniqueId(std::move(handle)));
args.rval().setNumber(PyEventLoop::AsyncHandle::getUniqueId(std::move(handle)));
return true;
Xmader marked this conversation as resolved.
Show resolved Hide resolved
}

// TODO (Tom Tang): move argument checks to the JavaScript side
static bool cancelByTimeoutId(JSContext *cx, unsigned argc, JS::Value *vp) {
using AsyncHandle = PyEventLoop::AsyncHandle;
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
JS::HandleValue timeoutIdArg = args.get(0);
double timeoutID = args.get(0).toNumber();

args.rval().setUndefined();

// silently does nothing when an invalid timeoutID is passed in
if (!timeoutIdArg.isInt32()) {
return true;
}

// Retrieve the AsyncHandle by `timeoutID`
int32_t timeoutID = timeoutIdArg.toInt32();
AsyncHandle *handle = AsyncHandle::fromId((uint32_t)timeoutID);
if (!handle) return true; // does nothing on invalid timeoutID

Expand Down
12 changes: 10 additions & 2 deletions tests/python/test_event_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,16 @@ def to_raise(msg):
# `setTimeout` should allow passing additional arguments to the callback, as spec-ed
assert 3.0 == await pm.eval("new Promise((resolve) => setTimeout(function(){ resolve(arguments.length) }, 100, 90, 91, 92))")
assert 92.0 == await pm.eval("new Promise((resolve) => setTimeout((...args) => { resolve(args[2]) }, 100, 90, 91, 92))")
# TODO (Tom Tang): test `setTimeout` setting delay to 0 if < 0
# TODO (Tom Tang): test `setTimeout` accepting string as the delay, coercing to a number like parseFloat
# test `setTimeout` setting delay to 0 if < 0
await asyncio.wait_for(pm.eval("new Promise((resolve) => setTimeout(resolve, 0))"), timeout=0.05)
await asyncio.wait_for(pm.eval("new Promise((resolve) => setTimeout(resolve, -10000))"), timeout=0.05) # won't be precisely 0s
# test `setTimeout` accepting string as the delay, coercing to a number.
# Number('100') -> 100, pass if the actual delay is > 90ms and < 150ms
await asyncio.wait_for(pm.eval("new Promise((resolve) => setTimeout(resolve, '100'))"), timeout=0.15) # won't be precisely 100ms
with pytest.raises(asyncio.exceptions.TimeoutError):
await asyncio.wait_for(pm.eval("new Promise((resolve) => setTimeout(resolve, '100'))"), timeout=0.09)
# Number("1 second") -> NaN -> delay turns to be 0s
await asyncio.wait_for(pm.eval("new Promise((resolve) => setTimeout(resolve, '1 second'))"), timeout=0.05) # won't be precisely 0s

# passing an invalid ID to `clearTimeout` should silently do nothing; no exception is thrown.
pm.eval("clearTimeout(NaN)")
Expand Down
Loading