-
Notifications
You must be signed in to change notification settings - Fork 633
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
Error "Cannot read property 'w' of null at iterateHttpRequests" on aborted connections #442
Comments
@MarkTiedemann could you run it with --reload? you seem to not use the last version of the http server. |
Still the same.
|
Related to this: #443 |
It seems to crash only at the end of the load test rather than because of the load. So probably when requests are being aborted. |
well that's a nice hint! On each run? I'll try to improve this. |
Yes, on each I did try to reproduce it making different combinations of aborted and partial requests from Node, but couldn't reproduce it yet. Maybe it's the combination of a certain load and aborted requests or something of that sort. |
After 9 runs no crash on my side. Running on OSX with deno 0.6. What's the env you're running on? |
macOS 10.14.4, deno 0.6.0. |
Did you run I just ran When I ran So I'm pretty sure that it has to do with how the connections are closed. When running with |
i've copied your example: So i'll work on with your new investigation. Thanks |
Cloned if (bufStateErr instanceof Error) {
// An error was thrown while parsing request headers.
+ console.error(bufStateErr);
await writeResponse(req.w, {
status: 400,
body: new TextEncoder().encode(`${bufStateErr.message}\r\n\r\n`)
});
}
The Not sure how to continue with this. Hope my investigation so far is helpful. |
In any case, we shouldn't try to send a HTTP 400 response when we can't read the status line of the request, right? |
Right. Because it can't be read as a HTTP request we don't have to answer. Maybe @ry or @piscisaureus can confirm? |
On a side note, having the HTTP server in the std lib makes it surprisingly easy to debug. ❤️ |
Here's a reliable reproduction: // main.ts
// Run with `deno run --allow-net main.ts`
import { serve } from "https://deno.land/std/http/server.ts";
async function main() {
for await (let req of serve(":80")) {
req.respond({});
}
}
main(); // repro.js
// Run with `node repro.js`
let net = require("net");
let client = net.createConnection({ port: 80 }, () => {
client.write("GET / HTTP/1.0\r\n\r\n");
client._destroy(new Error(), _ => {});
}); |
Is there any follow-up to this? |
…d#442) This change adds the `GET /api/users/[login]/notifications` REST API endpoint. Documentation will come in a follow-up PR. Towards denoland#439 and denoland#414
When I restarted the server and ran another load test, it crashed again. Might not be related to the high load, though.
The text was updated successfully, but these errors were encountered: