fix(ext/websocket): Ensure that errors are available after async response returns (#19642)

Fixes the WPT tests that test w/invalid codes. Also explicitly ignoring
some h2 tests to hopefully prevent flakes.

The previous changes to WebSocketStream introduced a bug where the close
errors were not made available if the `pull` method was re-entrant.
This commit is contained in:
Matt Mastracci 2023-06-29 07:24:01 -06:00 committed by GitHub
parent b6253370cc
commit 93b3ff0170
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 24 additions and 40 deletions

View file

@ -467,6 +467,7 @@ class WebSocket extends EventTarget {
default: { default: {
/* close */ /* close */
const code = kind; const code = kind;
const reason = code == 1005 ? "" : op_ws_get_error(rid);
const prevState = this[_readyState]; const prevState = this[_readyState];
this[_readyState] = CLOSED; this[_readyState] = CLOSED;
clearTimeout(this[_idleTimeoutTimeout]); clearTimeout(this[_idleTimeoutTimeout]);
@ -476,7 +477,7 @@ class WebSocket extends EventTarget {
await op_ws_close( await op_ws_close(
rid, rid,
code, code,
op_ws_get_error(rid), reason,
); );
} catch { } catch {
// ignore failures // ignore failures
@ -486,7 +487,7 @@ class WebSocket extends EventTarget {
const event = new CloseEvent("close", { const event = new CloseEvent("close", {
wasClean: true, wasClean: true,
code: code, code: code,
reason: op_ws_get_error(rid), reason,
}); });
this.dispatchEvent(event); this.dispatchEvent(event);
core.tryClose(rid); core.tryClose(rid);

View file

@ -242,8 +242,8 @@ class WebSocketStream {
}, },
}); });
const pull = async (controller) => { const pull = async (controller) => {
// Remember that this pull method may be re-entered before it has completed
const kind = await op_ws_next_event(this[_rid]); const kind = await op_ws_next_event(this[_rid]);
switch (kind) { switch (kind) {
case 0: case 0:
/* string */ /* string */
@ -266,17 +266,18 @@ class WebSocketStream {
core.tryClose(this[_rid]); core.tryClose(this[_rid]);
break; break;
} }
case 4: { case 1005: {
/* closed */ /* closed */
this[_closed].resolve(undefined); this[_closed].resolve({ code: 1005, reason: "" });
core.tryClose(this[_rid]); core.tryClose(this[_rid]);
break; break;
} }
default: { default: {
/* close */ /* close */
const reason = op_ws_get_error(this[_rid]);
this[_closed].resolve({ this[_closed].resolve({
code: kind, code: kind,
reason: op_ws_get_error(this[_rid]), reason,
}); });
core.tryClose(this[_rid]); core.tryClose(this[_rid]);
break; break;
@ -294,7 +295,8 @@ class WebSocketStream {
return pull(controller); return pull(controller);
} }
this[_closed].resolve(op_ws_get_error(this[_rid])); const error = op_ws_get_error(this[_rid]);
this[_closed].reject(new Error(error));
core.tryClose(this[_rid]); core.tryClose(this[_rid]);
} }
}; };

View file

@ -591,10 +591,8 @@ pub async fn op_ws_next_event(
Ok(val) => val, Ok(val) => val,
Err(err) => { Err(err) => {
// No message was received, socket closed while we waited. // No message was received, socket closed while we waited.
// Try close the stream, ignoring any errors, and report closed status to JavaScript. // Report closed status to JavaScript.
if resource.closed.get() { if resource.closed.get() {
let _ = state.borrow_mut().resource_table.close(rid);
resource.set_error(None);
return MessageKind::ClosedDefault as u16; return MessageKind::ClosedDefault as u16;
} }

View file

@ -567,8 +567,9 @@ function analyzeTestResult(
function reportVariation(result: TestResult, expectation: boolean | string[]) { function reportVariation(result: TestResult, expectation: boolean | string[]) {
if (result.status !== 0 || result.harnessStatus === null) { if (result.status !== 0 || result.harnessStatus === null) {
console.log(`test stderr:`); if (result.stderr) {
writeAllSync(Deno.stdout, new TextEncoder().encode(result.stderr)); console.log(`test stderr:\n${result.stderr}\n`);
}
const expectFail = expectation === false; const expectFail = expectation === false;
const failReason = result.status !== 0 const failReason = result.status !== 0
@ -611,6 +612,9 @@ function reportVariation(result: TestResult, expectation: boolean | string[]) {
for (const result of expectedFailedButPassed) { for (const result of expectedFailedButPassed) {
console.log(` ${JSON.stringify(result.name)}`); console.log(` ${JSON.stringify(result.name)}`);
} }
if (result.stderr) {
console.log("\ntest stderr:\n" + result.stderr);
}
console.log( console.log(
`\nfile result: ${ `\nfile result: ${
failedCount > 0 ? red("failed") : green("ok") failedCount > 0 ? red("failed") : green("ok")

View file

@ -6945,18 +6945,6 @@
"Create-extensions-empty.any.worker.html": true, "Create-extensions-empty.any.worker.html": true,
"Create-extensions-empty.any.worker.html?wpt_flags=h2": false, "Create-extensions-empty.any.worker.html?wpt_flags=h2": false,
"Create-extensions-empty.any.worker.html?wss": true, "Create-extensions-empty.any.worker.html?wss": true,
"Create-invalid-urls.any.html": true,
"Create-invalid-urls.any.html?wpt_flags=h2": true,
"Create-invalid-urls.any.html?wss": true,
"Create-invalid-urls.any.worker.html": true,
"Create-invalid-urls.any.worker.html?wpt_flags=h2": true,
"Create-invalid-urls.any.worker.html?wss": true,
"Create-non-absolute-url.any.html": false,
"Create-non-absolute-url.any.html?wpt_flags=h2": true,
"Create-non-absolute-url.any.html?wss": true,
"Create-non-absolute-url.any.worker.html": false,
"Create-non-absolute-url.any.worker.html?wpt_flags=h2": true,
"Create-non-absolute-url.any.worker.html?wss": true,
"Create-nonAscii-protocol-string.any.html": true, "Create-nonAscii-protocol-string.any.html": true,
"Create-nonAscii-protocol-string.any.html?wpt_flags=h2": true, "Create-nonAscii-protocol-string.any.html?wpt_flags=h2": true,
"Create-nonAscii-protocol-string.any.html?wss": true, "Create-nonAscii-protocol-string.any.html?wss": true,
@ -7030,12 +7018,6 @@
"Create-valid-url.any.worker.html": true, "Create-valid-url.any.worker.html": true,
"Create-valid-url.any.worker.html?wpt_flags=h2": false, "Create-valid-url.any.worker.html?wpt_flags=h2": false,
"Create-valid-url.any.worker.html?wss": true, "Create-valid-url.any.worker.html?wss": true,
"Create-wrong-scheme.any.html": true,
"Create-wrong-scheme.any.html?wpt_flags=h2": true,
"Create-wrong-scheme.any.html?wss": true,
"Create-wrong-scheme.any.worker.html": true,
"Create-wrong-scheme.any.worker.html?wpt_flags=h2": true,
"Create-wrong-scheme.any.worker.html?wss": true,
"Send-0byte-data.any.html": true, "Send-0byte-data.any.html": true,
"Send-0byte-data.any.html?wpt_flags=h2": false, "Send-0byte-data.any.html?wpt_flags=h2": false,
"Send-0byte-data.any.html?wss": true, "Send-0byte-data.any.html?wss": true,
@ -7212,19 +7194,17 @@
"close.any.html?wpt_flags=h2": false, "close.any.html?wpt_flags=h2": false,
"close.any.html?wss": true, "close.any.html?wss": true,
"close.any.worker.html?wpt_flags=h2": false, "close.any.worker.html?wpt_flags=h2": false,
"close.any.worker.html?wss": { "close.any.worker.html?wss": true,
"ignore": true
},
"constructor.any.html?wpt_flags=h2": false, "constructor.any.html?wpt_flags=h2": false,
"constructor.any.html?wss": true, "constructor.any.html?wss": true,
"constructor.any.worker.html?wpt_flags=h2": false, "constructor.any.worker.html?wpt_flags=h2": false,
"constructor.any.worker.html?wss": true, "constructor.any.worker.html?wss": true,
"abort.any.html?wpt_flags=h2": [ "abort.any.html?wpt_flags=h2": {
"abort after connect should do nothing" "ignore": true
], },
"abort.any.worker.html?wpt_flags=h2": [ "abort.any.worker.html?wpt_flags=h2": {
"abort after connect should do nothing" "ignore": true
], },
"backpressure-receive.any.worker.html?wpt_flags=h2": false "backpressure-receive.any.worker.html?wpt_flags=h2": false
} }
}, },

View file

@ -154,7 +154,6 @@ export async function runSingleTest(
harnessStatus = JSON.parse(line.slice(5)); harnessStatus = JSON.parse(line.slice(5));
} else { } else {
stderr += line + "\n"; stderr += line + "\n";
console.error(line);
} }
} }