fix(server) handle ignored body and node:http destroy #10268

cirospaciari · 2024-04-14T19:09:28Z

What does this PR do?

Fix: #10265
Fix: #10665

Documentation or TypeScript types (it's okay to leave the rest blank in this case)
Code changes

How did you verify your code works?

github-actions · 2024-04-14T19:36:06Z

❌ @cirospaciari 4 files with test failures on linux-x64-baseline:

test/integration/next-pages/test/dev-server.test.ts 1 failing
test/integration/next-pages/test/next-build.test.ts 1 failing
test/js/bun/http/serve-body-leak.test.ts 2 failing
test/js/node/http2/node-http2.test.js 3 failing

View test output

^{#5b31e884a7081cdbf3d8bb090b6b5dbc974f217d}

github-actions · 2024-04-14T19:36:13Z

❌ @cirospaciari 4 files with test failures on linux-x64:

test/integration/next-pages/test/dev-server.test.ts 1 failing
test/integration/next-pages/test/next-build.test.ts 1 failing
test/js/bun/http/serve-body-leak.test.ts 2 failing
test/js/node/http2/node-http2.test.js 2 failing

View test output

^{#5b31e884a7081cdbf3d8bb090b6b5dbc974f217d}

github-actions · 2024-04-14T19:37:35Z

❌ @cirospaciari 5 files with test failures on bun-darwin-aarch64:

test/js/bun/http/serve.test.ts 3 failing
test/js/bun/shell/leak.test.ts SIGTERM
test/js/bun/test/test-test.test.ts 1 failing
test/js/node/http2/node-http2.test.js 3 failing
test/js/web/streams/streams.test.js 1 failing

View test output

^{#5b31e884a7081cdbf3d8bb090b6b5dbc974f217d}

github-actions · 2024-04-14T19:48:30Z

❌ @cirospaciari 13 files with test failures on bun-windows-x86_64-haswell

test\cli\install\bunx.test.ts 1 failing
test\cli\install\registry\bun-install-registry.test.ts 5 failing
test\integration\next-pages\test\dev-server-ssr-100.test.ts 1 failing
test\integration\next-pages\test\dev-server.test.ts 1 failing
test\integration\next-pages\test\next-build.test.ts 1 failing
test\js\bun\glob\scan.test.ts 6 failing
test\js\bun\shell\bunshell.test.ts 2 failing
test\js\node\dns\node-dns.test.js 2 failing
test\js\node\http2\node-http2.test.js 1 failing
test\js\node\os\os.test.js 1 failing
test\js\third_party\esbuild\esbuild-child_process.test.ts 1 failing
test\js\web\websocket\websocket.test.js STATUS_SEVERITY_ERROR
test\js\web\workers\worker.test.ts SIGTERM

View test output

^{#5b31e884a7081cdbf3d8bb090b6b5dbc974f217d}

github-actions · 2024-04-14T19:54:42Z

❌ @cirospaciari 6 files with test failures on bun-darwin-x64:

test/cli/install/registry/bun-install-registry.test.ts 3 failing
test/js/bun/shell/leak.test.ts SIGTERM
test/js/bun/spawn/spawn.test.ts SIGTERM
test/js/bun/test/test-test.test.ts 1 failing
test/js/node/http2/node-http2.test.js 1 failing
test/js/web/timers/setTimeout.test.js 1 failing

View test output

^{#5b31e884a7081cdbf3d8bb090b6b5dbc974f217d}

Jarred-Sumner · 2024-04-15T11:58:35Z

src/bun.js/webcore/body.zig

+                            bun.default_allocator,
+                        );
+                    }
+                    readable.abort(global);


I think just calling abort should be enough

abort/cancel actually was only canceling the stream not the source, so I fixed that now.
onCancel actually only mark as done, not actually emits a error so I changed the test that I add to match this expectations.

github-actions · 2024-04-20T17:27:42Z

❌ @cirospaciari, your commit has failing tests :(

💪 2 failing tests Darwin AARCH64

test/js/bun/http/serve.test.ts 1 failing
test/js/node/http2/node-http2.test.js 1 failing

💻 2 failing tests Darwin x64

test/cli/install/bun-create.test.ts 2 failing
test/cli/install/registry/bun-install-registry.test.ts 3 failing

🐧💪 1 failing tests Linux AARCH64

test/js/bun/util/filesink.test.ts 2 failing

🐧🖥 1 failing tests Linux x64 baseline

test/js/node/http/node-http.test.ts SIGTERM

🪟💻 7 failing tests Windows x64 baseline

test/cli/install/bunx.test.ts 1 failing
test/integration/next-pages/test/dev-server-ssr-100.test.ts 1 failing
test/integration/next-pages/test/dev-server.test.ts 1 failing
test/integration/next-pages/test/next-build.test.ts 1 failing
test/js/bun/shell/bunshell.test.ts 1 failing
test/js/node/dns/node-dns.test.js 2 failing
test/js/node/fs/fs.test.ts 2 failing

🪟💻 11 failing tests Windows x64

test/cli/install/bun-install.test.ts 19 failing
test/cli/install/bunx.test.ts 1 failing
test/cli/install/migration/complex-workspace.test.ts 1 failing
test/cli/install/overrides.test.ts 1 failing
test/cli/install/registry/bun-install-registry.test.ts 4 failing
test/integration/next-pages/test/dev-server-ssr-100.test.ts 1 failing
test/integration/next-pages/test/dev-server.test.ts 1 failing
test/integration/next-pages/test/next-build.test.ts 1 failing
test/js/bun/shell/bunshell.test.ts 1 failing
test/js/node/dns/node-dns.test.js 2 failing
test/js/node/fs/fs.test.ts 2 failing

View logs

test/js/node/http/node-http.test.ts

Jarred-Sumner reviewed Apr 15, 2024

View reviewed changes

cirospaciari force-pushed the ciro/fix-server-body-deinit branch 2 times, most recently from a5891b6 to 8bb2d0f Compare April 20, 2024 16:39

cirospaciari force-pushed the ciro/fix-server-body-deinit branch from 3dc0a3c to d55f0c9 Compare April 30, 2024 17:14

cirospaciari added 20 commits April 30, 2024 22:37

if Locked and never consumed we still need to handle it

8a6c07d

memory leak test

db6b2a8

body leak tests

862ab39

ops

17f3ef7

rename lol

1ec7090

process for ignored streams

e0992c0

stream not consumed case

9402c19

undo

f2a356c

call cancel instead of done

8437630

fix peak memory usage

0425551

tweeaks

bc5ee9b

add a test for this fix too

d73ae5f

cleanup

a237677

echo test

1f5342b

expected in ignore

ee589a7

simplify

15957ef

cleanup

c484e80

make cancel actually cancel

6bf9383

more time

a3bff6a

oopsie

fc2e017

cirospaciari added 8 commits April 30, 2024 22:40

skip + time

8efc161

done actually mark source as done now

f479fc4

reduce test load

0d689a2

still slow

8320ebc

less flaky

1e9947f

tweaks

111c476

WIP end current request properly

634c197

oops

a7a735f

cirospaciari force-pushed the ciro/fix-server-body-deinit branch from fdbe9e2 to a7a735f Compare May 1, 2024 03:39

cirospaciari added 3 commits May 1, 2024 00:44

add comment

3c425fa

revert

35ae823

fix http destroy

552f9b1

cirospaciari changed the title ~~fix(server) handle ignored body~~ fix(server) handle ignored body and node:http destroy May 1, 2024

cirospaciari marked this pull request as ready for review May 1, 2024 05:21

fix abort call

58ae8db

oguimbal reviewed May 1, 2024

View reviewed changes

test/js/node/http/node-http.test.ts Outdated Show resolved Hide resolved

opsie make this test actual be a test

87d4aad

cirospaciari requested a review from Jarred-Sumner May 1, 2024 19:21

change test hostname

64ba7f3

Jarred-Sumner merged commit 79c7abe into main May 1, 2024
40 of 52 checks passed

Jarred-Sumner deleted the ciro/fix-server-body-deinit branch May 1, 2024 23:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(server) handle ignored body and node:http destroy #10268

fix(server) handle ignored body and node:http destroy #10268

cirospaciari commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

Jarred-Sumner Apr 15, 2024

cirospaciari Apr 15, 2024 •

edited

Loading

github-actions bot commented Apr 20, 2024 •

edited

Loading

fix(server) handle ignored body and node:http destroy #10268

fix(server) handle ignored body and node:http destroy #10268

Conversation

cirospaciari commented Apr 14, 2024 • edited Loading

What does this PR do?

How did you verify your code works?

github-actions bot commented Apr 14, 2024 • edited Loading

github-actions bot commented Apr 14, 2024 • edited Loading

github-actions bot commented Apr 14, 2024 • edited Loading

github-actions bot commented Apr 14, 2024 • edited Loading

github-actions bot commented Apr 14, 2024 • edited Loading

Jarred-Sumner Apr 15, 2024

Choose a reason for hiding this comment

cirospaciari Apr 15, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 20, 2024 • edited Loading

💪 2 failing tests Darwin AARCH64

💻 2 failing tests Darwin x64

🐧💪 1 failing tests Linux AARCH64

🐧🖥 1 failing tests Linux x64 baseline

🪟💻 7 failing tests Windows x64 baseline

🪟💻 11 failing tests Windows x64

cirospaciari commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

github-actions bot commented Apr 14, 2024 •

edited

Loading

cirospaciari Apr 15, 2024 •

edited

Loading

github-actions bot commented Apr 20, 2024 •

edited

Loading