Skip to content

Change fish path addition command - #207

Merged
Jarred-Sumner merged 1 commit into
oven-sh:mainfrom
addy:fish
Jul 5, 2022
Merged

Jarred-Sumner merged 1 commit into
oven-sh:mainfrom
addy:fish

Conversation

@addy

@addy addy commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

I noticed in the README that the fish_add_path function is mentioned. It's also been a function that's been around since fish 3.2.0 (March 2021) so I think this should be a relatively sane change.

@Jarred-Sumner
Jarred-Sumner merged commit 44ee4ca into oven-sh:main Jul 5, 2022
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

Thank you

Comment thread src/cli/install.sh
if test -f $HOME/.config/fish/config.fish; then
echo -e "\n# bun\nset -Ux BUN_INSTALL \"$bun_install\"" >>"$HOME/.config/fish/config.fish"
echo -e "set -px --path PATH \"$bin_dir\"\n" >>"$HOME/.config/fish/config.fish"
echo -e "fish_add_path \"$bin_dir\"\n" >>"$HOME/.config/fish/config.fish"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that these two changes are not quite equivalent.

By default, fish_add_path modifies $fish_user_paths, a universal variable. If you just want to modify $PATH by default, this should be done with fish_add_path -P ….

Jarred-Sumner pushed a commit that referenced this pull request Jul 23, 2026
#35255)

`test/js/bun/http/serve-protocols.test.ts` has been going red on main
(build 78445 darwin-x64 hard, 78462 debian-11-aarch64 hard, plus
78300/78419 with retries), always as

```
error: HTTP3StreamReset fetching "https://127.0.0.1:<port>/echo"
✗ Bun.serve over http/3 > POST echo 1000000 bytes [20169.37ms]
```

Reproduced on Linux by looping the file: the h3 subset alone fails about
17 of 100 runs.

## Cause

The ten concurrent h3 tests share one lsquic client engine and one
unconnected UDP socket. `bsd_create_udp_socket()` sets `IP_RECVERR` on
every UDP socket (for `node:dgram`'s error surfacing, #28827), including
QUIC's. When a finished test `proc.kill()`s its server, the client
session is still in the engine and keeps scheduling retransmits /
`NEW_CONNECTION_ID` to an unbound port. With `IP_RECVERR` on, the
resulting ICMP port-unreachable is queued on the shared socket and the
next `sendmmsg` returns `-1 ECONNREFUSED`, even though that call is
sending a datagram to a live peer.

`us_quic_packets_out` reports that as a short return, lsquic clears
`ENPUB_CAN_SEND` for the whole engine and only its one-second
`resume_sending_at` failsafe re-enables it. With several dead sessions
generating ICMPs, every failsafe retry fails the same way and the live
1MB upload never advances; the 20s in CI is two idle-timeout rounds
through `retry_or_fail`.

While tracing that I also found an unsigned underflow in lsquic's
`send_batch` requeue loop: when the first unsent spec in a batch
coalesces multiple packets (`pack_off[0] == 0`, `iovlen > 1`), `end =
&batch->packets[off - 1]` indexes with `UINT_MAX` and only the last
packet of the coalesced group is returned to the connection. The earlier
ones are the INIT ACK and the HSK CRYPTO carrying the client Finished,
so the peer can never complete the handshake. This is the same hang
reached from a different direction (real EAGAIN backpressure instead of
stale ICMP).

## Fix

- `IP_RECVERR` is now opt-in via `LIBUS_UDP_LINUX_RECVERR`, set by
`us_create_udp_socket` when a `recv_error_cb` is provided. `node:dgram`
always passes one and keeps the option; QUIC passes `NULL` and no longer
gets it. This matches libuv's `UV_UDP_LINUX_RECVERR` gating that `bsd.c`
already cited.
- `us_quic_packets_out()` retries once on a non-`EAGAIN`/`ENOBUFS` send
failure before reporting a short return, so a stale `sk_err` that does
surface cannot pause the engine. Both the `sendmmsg` and per-packet
paths now go through `US_FAULT_CHECK(US_FAULT_SENDMSG, ...)` so the
short-return path is reachable from tests.
- `patches/lsquic/requeue-unsent-coalesced.patch` rewrites the requeue
loop's bounds as `[off, off+count)` so every packet in an unsent
coalesced datagram is returned to the connection. The same underflow is
present in upstream lsquic master; I will open a PR there separately.
- `serve-protocols.test.ts` now stops each fixture server gracefully on
stdin close (`server.stop(true)`), matching `serve-http3.test.ts`, so
the pooled client session sees `CONNECTION_CLOSE` instead of leaving the
engine retransmitting to unbound ports.
- `test/js/web/fetch/fetch-http3-syscall-fault.test.ts` injects `EAGAIN`
on the coalesced handshake datagram (the `pack_off[0]==0`, `iovlen>1`
spec the lsquic patch fixes), a one-shot `ECONNREFUSED` that the
retry-once branch consumes, and a burst of `EAGAIN` that the `on_drain`
path recovers from.

## Verification

Release build, looped:

| | before | after |
| --- | --- | --- |
| `serve-protocols -t "http/3"` | 17/100 fail | 2/100 fail |
| `serve-protocols` (full) | 7/100 fail | 4/200 fail |

Debug+ASAN: `serve-protocols`, `serve-http3` (46), `fetch-http3-client`
(52), `fetch-http3-adversarial` (29), `fetch-http3-syscall-fault` (3)
and `dgram.test.ts` all pass, 211 tests total.

The residual ~1-2% is a separate pre-existing bug (the client's 36-byte
HSK CRYPTO is buffered but never flushed when `drain_send_body` writes
the whole 1MB body synchronously from `on_stream_open`); I've handed
that off as its own issue. With CI's retry it is well under the flake
threshold.

### Gate note

The fault-injection hook that makes the new test deterministic lives in
`packages/bun-usockets/src/quic.c`, so `git stash -- src/ packages/`
removes it along with the fix and the fault never fires. The lsquic
piece lives in `patches/` and `scripts/`, which the stash does not
touch. That means a single stashed run passes (no fault, no stall) and a
single unstashed run passes (fault fires, fix handles it), and the gate
cannot distinguish them mechanically. The 300-iteration probe above is
the evidence; the fault-injection tests pin the behavior going forward.

<details>
<summary>lsquic debug trace of the stall</summary>

```
engine: packets out returned 0 (out of 1)
[C919…] event: unsent packet #15 ACK_FREQUENCY, size 36
[C919…] sendctl: packet #15 has been delayed
engine: send_packets_out: sent 0 packets
…                                                    <- no "can send again"; nothing for 1s
engine: failsafe activated: resume sending packets again after timeout
engine: packets out returned 0 (out of 10)           <- fails again, live conn's #207 included
```

and for the underflow, a batch with `pack_off[0]=0`, `iovlen[0]=3`:

```
engine: packets out returned 0 (out of 2)
event: unsent packet #3 ACK PADDING, size 1059
event: unsent packet #4 ACK CRYPTO, size 87
event: unsent packet #5 NEW_CONNECTION_ID, size 54
event: unsent packet #6 STREAM, size 114
sendctl: packet #6 has been delayed
sendctl: packet #5 has been delayed
…                                                    <- #3 and #4 never requeued
[WARN] sendctl: send history gap 2 - 5
```
</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 1 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/web/fetch/fetch-http3-syscall-fault.test.ts

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants