supabase / edge-runtime

A server based on Deno runtime, capable of running JavaScript, TypeScript, and WASM services.
MIT License
691 stars 62 forks source link

feat: reintroduce `DuplexStream` #327

Closed nyannyacha closed 6 months ago

nyannyacha commented 7 months ago

What kind of change does this PR introduce?

Bug fix, Enhancement

Description

This PR is a continuation of PR-320. Please see PR-320 for background on the PR.

Major changes compared by PR-320

Makes the timing of pipe closure is adjustable within a separately created tokio task, so that the stream is not closed in the middle of a response.

In the case of DuplexStream, if either pair is dropped, the stream immediately goes into an unwritable state.

These streams must remain writable for outside responses, but the previous PR did not. So this issue was immediately exposed by the flush behavior of libraries like hyper.

With the current implementation, which requires a Tokio task to be created every time a stream is dropped, I get a small performance hit compared to PR-320, but I believe we can replace it with an efficient implementation in the next iteration.

Notes on Minor changes

ConnSync has been removed and replaced with CancellationToken.

This was a refactoring to simplify the structure, and while the performance impact of the replacement has not been carefully evaluated, it does not appear to have changed much.

Stream of request body must be cancelable from errors or exceptions by the worker.

Sometimes the request body is not fully consumed by the worker due to circumstances such as worker resource limit due to a multipart request.

Previously, sb_workers did not handle these circumstances properly, leaving the end requestor in a situation where they would receive no response indefinitely. (Runtime also had resource leaks due to unresolved promises)

This PR introduces a routine that tries to cancel the request body stream after checking for a response or exception from the worker.

Graceful shutdown by SIGWINCH is now behind the base/termination-signal-ext feature flag.

Given the nature of devs frequently changing the terminal size, SIGWINCH has been gated to only be enabled behind a specific feature flag.

github-actions[bot] commented 6 months ago

:tada: This PR is included in version 1.48.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: