diff --git a/README.md b/README.md index 73770f0..4116b2b 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ To move the pin, move both: ```sh git -C rayforce-sys/vendor/rayforce fetch --tags -git -C rayforce-sys/vendor/rayforce checkout v2.6.0 +git -C rayforce-sys/vendor/rayforce checkout v2.6.1 git add rayforce-sys/vendor/rayforce git -C rayforce-sys/vendor/rayforce rev-parse --short=7 HEAD # CORE_COMMIT diff --git a/docs/docs/content/CHANGELOG.md b/docs/docs/content/CHANGELOG.md index fc72e41..e5e1a33 100644 --- a/docs/docs/content/CHANGELOG.md +++ b/docs/docs/content/CHANGELOG.md @@ -29,7 +29,7 @@ All notable changes to `rayforce` are documented here. This project adheres to ### Changed -- **The vendored core is v2.6.0 and `rayforce-q` is 2.1.1** (from v2.5.8 and +- **The vendored core is v2.6.1 and `rayforce-q` is `1eabaf4`** (from v2.5.8 and 2.0.0). The core now recognises in-band nulls at construction, which changes what a vector built from a raw buffer reports: `Value::vec(&[1i64, i64::MIN, 3])` answers `is_null_at(1)` and `get(1)` returns the null singleton, where before @@ -43,6 +43,64 @@ All notable changes to `rayforce` are documented here. This project adheres to longer describe a "null bitmap": nulls are sentinels behind a `HAS_NULLS` fast-path hint. +- **A failed `TcpClient::connect` says why.** Every negative return from the + core collapsed into `connect to {host}:{port} failed`, which reads the same + whether nothing was listening, the password was wrong, or the peer speaks a + wire version this build would misparse every atom of. The core distinguishes + six causes — v2.6.1 added two of them — so the message now ends in + `connection refused`, `authentication failed`, `wire version mismatch`, + `timed out`, or the OS error text, the same shape `QConnection::connect_with` + has always used for its own three codes. Two of those read less plainly than + they look: `timed out` also covers a server that is alive but busy inside a + long evaluation, because the core folds `EAGAIN`/`EWOULDBLOCK` in with + `ETIMEDOUT`, and a host that fails to resolve surfaces as `No route to host`, + which is the `errno` the core stamps on that failure. `rayforce-sys` gained + `RAY_IPC_ERR_*` constants for the codes, mirroring the `Q_ERR_*` ones — the + public header declares no contract for them, so they are maintained by hand + against `connect_fail_code()` in the core, and an unrecognised code still + falls through to `connection refused`. One cause stays out of reach: + `server requires authentication` needs a null password, and an empty `&str` + arrives as a valid pointer to an empty string. + +- **`QConnection` no longer takes the process down when a q peer disappears.** + The `rayforce-q` pin moves off the 2.1.1 tag to `1eabaf4` — six fixes to + `q.c`, the one file of that repo this crate compiles, and no tag carries them + yet. Writing to a closed peer used to raise `SIGPIPE`, whose default + disposition kills the process: a library has no business doing that to its + host, and `q_send_all` now passes `MSG_NOSIGNAL` (`SO_NOSIGPIPE` on the BSDs). + A reply is accepted only when the frame says it is one, instead of any message + type being decoded as the answer to the request in flight. A q identity reply + (`::`, what an assignment answers) decodes to the null object rather than + failing the exchange with "unsupported wire type". A native `RAY_DICT` result + now encodes, where the serializer had no branch for it and gave up. And a + malformed reply whose decode left trailing bytes freed an error object through + `ray_release` rather than `ray_error_free`. `q.h` is untouched, so nothing in + this crate's FFI declarations moves. + +- **`count (distinct …)` counts a null as a value inside `by:` groups.** The + v2.6.1 core retires the per-group kernel's null-skipping arm: a grouped + `count distinct` over a null-bearing column now answers one more than it did, + matching what the ungrouped form has always returned. The old convention was + not even self-consistent — the serial, partitioned and per-group-buffer + kernels disagreed, so the answer moved with the row count, the group count and + the core count. Nothing in this crate's surface changes; the numbers coming + back from `Select::by(…)` do. + +- **The rest of the v2.6.1 engine deltas that reach this crate.** `.csv.read` + also accepts Rayfall's dotted temporal spellings (`2024.01.02`, + `2024.01.02D01:02:03`) alongside the ISO forms the CSV writer emits, so a file + written by `dump` round-trips. `if` with a null branch no longer writes an + ordinary huge number where a null belongs — a null atom stays null across + widths, and an `F64` past the `int64` range narrows to the integer null rather + than an undefined cast. A periodic timer that overruns its period re-arms at + the next deadline instead of replaying every fire it missed, and a failing + callback prints `timer : error: : `. The engine binary now + exits 1 when `-p` cannot bind, rather than running the script and exiting 0 + with no listener — relevant to `tests/ipc.rs`, which spawns one. `update + where:` and `upsert` write in place on a *named* flat table; the builders here + pass a table value rather than a quoted name, so they keep taking the copy + path and are unaffected. + - **The submodules are addressed over SSH.** `.gitmodules` now points at `git@github.com:RayforceDB/rayforce.git` and `rayforce-q.git`. An existing clone picks the change up with `git submodule sync --recursive`; CI needs diff --git a/docs/docs/content/documentation/ipc.md b/docs/docs/content/documentation/ipc.md index fc02cc5..57a208c 100644 --- a/docs/docs/content/documentation/ipc.md +++ b/docs/docs/content/documentation/ipc.md @@ -42,6 +42,16 @@ Runtime::scope(|_rt| { # Ok::<(), rayforce::RayError>(()) ``` +A failure names its cause rather than reporting "failed": + +| Message ends with | What happened | +| --- | --- | +| `connection refused` | Nothing is listening, or the core refused before reaching the peer | +| `authentication failed` | The server rejected the credentials | +| `wire version mismatch` | The peer speaks a serialization version this build would misparse | +| `timed out` | No answer within the 5s budget — including from a server that is up but busy inside a long evaluation | +| an OS error, e.g. `No route to host (os error 113)` | Anything else the socket layer reported; a host that fails to resolve arrives here | + The connection is **closed on drop** — when the `TcpClient` goes out of scope the socket is released. You can also close it explicitly with `client.close()`. diff --git a/docs/docs/content/get-started/installation.md b/docs/docs/content/get-started/installation.md index c4be7f5..22d911a 100644 --- a/docs/docs/content/get-started/installation.md +++ b/docs/docs/content/get-started/installation.md @@ -92,11 +92,11 @@ Moving the pin means moving the submodule and the constants together: ```sh # 1. Move the submodule to the new tag. git -C rayforce-sys/vendor/rayforce fetch --tags -git -C rayforce-sys/vendor/rayforce checkout v2.6.0 +git -C rayforce-sys/vendor/rayforce checkout v2.6.1 git add rayforce-sys/vendor/rayforce # 2. Read back the values build.rs must stamp. -git -C rayforce-sys/vendor/rayforce describe --tags --exact-match # -> v2.6.0 +git -C rayforce-sys/vendor/rayforce describe --tags --exact-match # -> v2.6.1 git -C rayforce-sys/vendor/rayforce rev-parse --short=7 HEAD # -> e.g. 1a2b3c4 ``` @@ -104,7 +104,7 @@ Then edit `rayforce-sys/build.rs` to match — `CORE_VERSION` is the tag without its leading `v`: ```rust -const CORE_VERSION: &str = "2.6.0"; +const CORE_VERSION: &str = "2.6.1"; const CORE_COMMIT: &str = "1a2b3c4"; ``` diff --git a/rayforce-sys/build.rs b/rayforce-sys/build.rs index 639c05c..7c8c733 100644 --- a/rayforce-sys/build.rs +++ b/rayforce-sys/build.rs @@ -24,7 +24,7 @@ use std::process::Command; /// /// Must match the tag `vendor/rayforce` is pinned to. CI asserts the two agree; /// see the "Check vendored core pin" step in `.github/workflows/ci.yml`. -const CORE_VERSION: &str = "2.6.0"; +const CORE_VERSION: &str = "2.6.1"; /// Commit the `vendor/rayforce` submodule is pinned to, stamped alongside /// [`CORE_VERSION`]. Also checked by CI's "Check vendored core pin" step. @@ -35,7 +35,7 @@ const CORE_VERSION: &str = "2.6.0"; /// under OUT_DIR, an unset value does not fall back to "unknown" — it silently /// reports the HEAD of whatever unrelated repository happens to enclose the /// build directory. -const CORE_COMMIT: &str = "b3e9aa1"; +const CORE_COMMIT: &str = "a99a019"; /// Warning flags for the vendored core build — the core's own `WARNS` /// (`Makefile:30`) minus `-Werror`. Consumers compile this with whatever diff --git a/rayforce-sys/src/lib.rs b/rayforce-sys/src/lib.rs index f7814cc..faa076d 100644 --- a/rayforce-sys/src/lib.rs +++ b/rayforce-sys/src/lib.rs @@ -57,3 +57,28 @@ extern "C" { pub const Q_ERR_SOCKET: ::std::os::raw::c_int = -1; pub const Q_ERR_HANDSHAKE: ::std::os::raw::c_int = -2; pub const Q_ERR_TIMEOUT: ::std::os::raw::c_int = -3; + +/// `ray_ipc_connect` failure codes. +/// +/// The public header declares the function with no contract at all +/// (`include/rayforce.h`); this mirrors the comment on the private +/// `src/core/ipc.h` declaration and, for [`RAY_IPC_ERR_OS`], the +/// `connect_fail_code()` classifier in `src/core/ipc.c` that the header's +/// comment predates. Hand-maintained against a core bump, like `Q_ERR_*`. +/// +/// [`RAY_IPC_ERR_REFUSED`] is the documented name for -1, but the core also +/// returns it for a missing poll, a credential buffer overflow and a failed +/// poll registration — treat it as the catch-all it is. +pub const RAY_IPC_ERR_REFUSED: i64 = -1; +/// The server demands credentials and the caller supplied no password. +pub const RAY_IPC_ERR_AUTH_REQUIRED: i64 = -2; +/// The server rejected the credentials. +pub const RAY_IPC_ERR_AUTH_FAILED: i64 = -3; +/// The peer speaks a different serialization wire version. +pub const RAY_IPC_ERR_WIRE_VERSION: i64 = -4; +/// No answer within the connect/handshake budget. The core folds `EAGAIN` and +/// `EWOULDBLOCK` in here too, so this also covers a live server that is busy +/// inside a long evaluation. +pub const RAY_IPC_ERR_TIMEOUT: i64 = -5; +/// Some other OS error; the core leaves `errno` holding it. +pub const RAY_IPC_ERR_OS: i64 = -6; diff --git a/rayforce-sys/vendor/rayforce b/rayforce-sys/vendor/rayforce index b3e9aa1..a99a019 160000 --- a/rayforce-sys/vendor/rayforce +++ b/rayforce-sys/vendor/rayforce @@ -1 +1 @@ -Subproject commit b3e9aa187e93a97a0c04fd9f85f63b65e3c67905 +Subproject commit a99a019e796f9632cabfb8f351164c8ab261c125 diff --git a/rayforce-sys/vendor/rayforce-q b/rayforce-sys/vendor/rayforce-q index c35ed3c..1eabaf4 160000 --- a/rayforce-sys/vendor/rayforce-q +++ b/rayforce-sys/vendor/rayforce-q @@ -1 +1 @@ -Subproject commit c35ed3c574a05c233a674dd12a5dc7703d2a00b1 +Subproject commit 1eabaf47c13ae695920f7839f3e8797c3379b698 diff --git a/rayforce/src/ipc.rs b/rayforce/src/ipc.rs index 650fdf6..926c6f6 100644 --- a/rayforce/src/ipc.rs +++ b/rayforce/src/ipc.rs @@ -8,6 +8,7 @@ use crate::error::{check, materialize, RayError, Result}; use crate::runtime::assert_on_runtime_thread; use crate::value::Value; use rayforce_sys as sys; +use std::borrow::Cow; use std::ffi::CString; use std::marker::PhantomData; @@ -56,6 +57,26 @@ pub struct TcpClient { impl TcpClient { /// Connect to `host:port`, optionally authenticating. Requires a live /// [`crate::Runtime`]. + /// + /// A failure names its cause: the server refused the connection, demanded + /// credentials, rejected the ones given, speaks a different wire version, + /// did not answer in time, or failed with some other OS error (whose text + /// is included). Two of those read less plainly than they look: + /// + /// - `timed out` also covers a server that is alive and listening but busy + /// inside a long evaluation — the core folds `EAGAIN`/`EWOULDBLOCK` in + /// with `ETIMEDOUT`, because from here they are the same silence. The + /// budget is the core's 5s default; this call does not set one. + /// - The OS error text behind the last case comes from `errno`, which the + /// core does not reliably bridge from `WSAGetLastError()` on Windows. + /// Only the Unix targets are built and tested here. A `host` that fails + /// to resolve arrives through it as `No route to host`, because that is + /// the `errno` the core stamps on a name-resolution failure. + /// + /// `server requires authentication` is not reachable through this method: + /// the core raises it when handed a null password, and an empty `password` + /// still arrives as a valid pointer to an empty string, which the server + /// rejects as a bad credential instead. pub fn connect(host: &str, port: u16, user: &str, password: &str) -> Result { assert_on_runtime_thread("TcpClient::connect"); let host_c = CString::new(host).map_err(|_| RayError::binding("host contains NUL"))?; @@ -68,8 +89,23 @@ impl TcpClient { let handle = sys::ray_ipc_connect(host_c.as_ptr(), port, user_c.as_ptr(), pass_c.as_ptr(), 0); if handle < 0 { + // Borrowed for the fixed reasons, owned only for the errno + // one, so the common paths do not allocate. Nothing runs + // between the call returning and the errno read but this + // match, which cannot disturb it. + let reason: Cow<'static, str> = match handle { + sys::RAY_IPC_ERR_AUTH_REQUIRED => "server requires authentication".into(), + sys::RAY_IPC_ERR_AUTH_FAILED => "authentication failed".into(), + sys::RAY_IPC_ERR_WIRE_VERSION => "wire version mismatch".into(), + sys::RAY_IPC_ERR_TIMEOUT => "timed out".into(), + sys::RAY_IPC_ERR_OS => std::io::Error::last_os_error().to_string().into(), + // RAY_IPC_ERR_REFUSED is the core's catch-all as much as + // it is its "refused", and a future core may return a code + // this build has never heard of. + _ => "connection refused".into(), + }; return Err(RayError::binding(format!( - "connect to {host}:{port} failed" + "connect to {host}:{port} failed: {reason}" ))); } Ok(TcpClient { diff --git a/rayforce/tests/ipc.rs b/rayforce/tests/ipc.rs index 7124a7c..e56659b 100644 --- a/rayforce/tests/ipc.rs +++ b/rayforce/tests/ipc.rs @@ -4,7 +4,7 @@ //! a free port, connect, and exchange queries. Skips if no binary is available. use rayforce::{Runtime, TcpClient}; -use std::io::Write; +use std::io::{Read, Write}; use std::net::{TcpListener, TcpStream}; use std::path::PathBuf; use std::process::{Child, Command}; @@ -133,11 +133,60 @@ fn client_reports_server_error() { } #[test] -fn connect_failure_is_an_error() { +fn connect_failure_names_its_cause() { Runtime::scope(|_rt| { - // Nothing listening on this port. + // free_port() hands back a port it has already released, so nothing is + // listening and the kernel answers ECONNREFUSED — the one connect + // failure reachable without a peer to misbehave for us. let port = free_port(); - assert!(TcpClient::connect("127.0.0.1", port, "", "").is_err()); + let e = TcpClient::connect("127.0.0.1", port, "", "") + .err() + .expect("connect should have failed"); + let msg = e.to_string(); + assert!( + msg.contains("connection refused"), + "expected a named cause, got {msg:?}" + ); + assert!( + msg.contains(&port.to_string()), + "message lost the port: {msg:?}" + ); + Ok(()) + }) + .unwrap(); +} + +/// Bind a listener that completes the TCP accept, reads the client's two-byte +/// handshake, and answers with a wire version the core cannot speak. Returns +/// the port. `RAY_SERDE_WIRE_VERSION` is 3, so 0xFF is reliably wrong. +fn spawn_wrong_version_peer() -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + std::thread::spawn(move || { + if let Ok((mut sock, _)) = listener.accept() { + let mut hs = [0u8; 2]; + let _ = sock.read_exact(&mut hs); + let _ = sock.write_all(&[0xFFu8, 0x00]); + } + }); + port +} + +#[test] +fn connect_reports_a_wire_version_mismatch() { + // Distinct from a refusal: the peer is listening and answers, it just + // speaks a protocol this build would misparse every atom of. Collapsing + // the two into "failed" is what this test exists to prevent. + Runtime::scope(|_rt| { + let port = spawn_wrong_version_peer(); + let e = TcpClient::connect("127.0.0.1", port, "", "") + .err() + .expect("connect should have failed"); + let msg = e.to_string(); + assert!( + msg.contains("wire version mismatch"), + "expected a wire-version cause, got {msg:?}" + ); Ok(()) }) .unwrap();