> {
+ if self.sessions.read().await.get(alias)?.fs.is_alive() {
+ return self.sessions.read().await.get(alias).cloned();
+ }
+ let mut sessions = self.sessions.write().await;
+ // Checked again under the write lock: another request may have replaced it since, and
+ // evicting the replacement would start this over.
+ if sessions.get(alias).is_some_and(|s| !s.fs.is_alive()) {
+ sessions.remove(alias);
+ }
+ sessions.get(alias).cloned()
+ }
+
/// Open every enabled host, at once, and say which ones would not.
///
/// At once rather than in turn: these are independent ssh handshakes, and in sequence six
@@ -2227,6 +2591,29 @@ impl Origin {
.collect();
sites.sort();
+ // Declared and not connected. Listed rather than left out: a reader who can see
+ // `panza` in their config file and not on the page whose whole job is saying what is
+ // served has been told nothing, and that is the silent failure under another name.
+ let (trouble, stopped) = (self.trouble.read().await, self.stopped.read().await);
+ let mut down: Vec<(String, String)> = self
+ .declared
+ .iter()
+ .filter(|(name, _)| !sites.iter().any(|(open, ..)| open == *name))
+ .map(|(name, d)| {
+ let why = if stopped.contains(name) {
+ "stopped from the dashboard; a restart serves it again".to_string()
+ } else {
+ trouble
+ .get(name)
+ .map(|t| t.why.clone())
+ .unwrap_or_else(|| format!("{} has not answered", d.host))
+ };
+ (name.clone(), why)
+ })
+ .collect();
+ down.sort();
+ drop((trouble, stopped));
+
let mut s = String::from(
"\
\
@@ -2284,10 +2671,29 @@ impl Origin {
s.push_str("");
}
+ if !down.is_empty() {
+ s.push_str("not connected
");
+ }
+
s.push_str(
"Each of these is its own origin, which is what this page is a \
- list of. Serving a host and stopping one happen in the dashboard: those go \
- through the daemon's control API, and no page reaches that.
\
+ list of. Opening one that is not connected is what retries it. Serving a host and \
+ stopping one happen in the dashboard: those go through the daemon's control API, \
+ and no page reaches that.\
",
);
s
@@ -3152,7 +3558,11 @@ mod tests {
}
async fn origin_with_cache(remote: FakeRemote, cache: Cache) -> Origin {
- let fs = remote.spawn().await;
+ origin_over(remote.spawn().await, cache)
+ }
+
+ /// The same origin, over a connection the caller made -- so it can also end it.
+ fn origin_over(fs: SftpFs, cache: Cache) -> Origin {
let mut sessions = HashMap::new();
sessions.insert(
"docs".to_string(),
@@ -3179,6 +3589,22 @@ mod tests {
reachable: RwLock::new(reachable::Set::default()),
handshakes: Handshakes::default(),
dashboard: RwLock::new(None),
+ // Declared as well as connected, because that is what production looks like and
+ // because the reconnect path reads it: an alias whose session dies has to be
+ // re-dialled from somewhere, and a test whose `docs` was connected but never
+ // declared would exercise the half of `session_for` that cannot retry.
+ declared: HashMap::from([(
+ "docs".to_string(),
+ Declared {
+ host: "nowhere".to_string(),
+ base: Some("/srv".to_string()),
+ named: Named::InTheFile,
+ dialling: tokio::sync::Mutex::new(()),
+ },
+ )]),
+ trouble: RwLock::new(HashMap::new()),
+ stopped: RwLock::new(HashSet::new()),
+ cooldown: DIAL_COOLDOWN,
}
}
@@ -3192,6 +3618,14 @@ mod tests {
b.body(Empty::new()).expect("request builds")
}
+ fn get_on(alias: &str, path: &str) -> Request> {
+ Request::builder()
+ .uri(format!("http://{alias}.ssh-browser{path}"))
+ .header(HOST, format!("{alias}.ssh-browser"))
+ .body(Empty::new())
+ .expect("request builds")
+ }
+
async fn trips(origin: &Origin) -> u64 {
origin
.sessions
@@ -4690,22 +5124,171 @@ mod tests {
/// A setting that took effect at the next restart is indistinguishable from one that did
/// not work, and in this direction it is worse than confusing: a host still answering after
/// you switched it off is an ssh session you believe you have given back.
+ /// The failure this whole path exists to end.
+ ///
+ /// A declared alias whose ssh will not come up used to be fatal at startup and, once past
+ /// that, indistinguishable from a name nobody had written down. Now it is a gateway that
+ /// did not answer, said in the status code, and the next request asks again.
#[tokio::test]
- async fn disabling_a_host_closes_it_now() {
- let origin = origin_with(one_page()).await;
- // `docs` is the alias the test origin serves. Naming it here needs no ssh_config, but
- // the route checks ssh_config before it does anything -- so this asserts the closing
- // through the piece that does not need a network, and the gate above covers the rest.
- assert!(origin.session("docs").await.is_some());
+ async fn an_alias_whose_host_is_down_is_still_an_alias() {
+ let mut origin = origin_with(one_page()).await;
+ // No cooldown, so the second request below reaches the dial rather than the memory of
+ // the first. The cooldown itself is the next test.
+ origin.cooldown = Duration::ZERO;
+ // The declared host is `nowhere`, so re-dialling it fails without a network.
origin.sessions.write().await.remove("docs");
+
+ let res = origin.handle(get("/a.html", None)).await;
+ assert_eq!(
+ res.status(),
+ StatusCode::BAD_GATEWAY,
+ "a declared alias that will not connect is 502, not 404"
+ );
+ assert!(
+ origin.trouble.read().await.contains_key("docs"),
+ "the reason has to outlive the request that found it"
+ );
+
+ // Not latched. Asked again, the dial happens again -- a daemon that took the first
+ // failure as final would pass every check above and still need restarting.
+ //
+ // `at` is the evidence and the status is not: a second 502 is what a re-dial and a
+ // remembered failure both look like from outside, so a test that only read the status
+ // would pass whichever this did.
+ let first = origin.trouble.read().await["docs"].at;
+ assert_eq!(
+ origin.handle(get("/a.html", None)).await.status(),
+ StatusCode::BAD_GATEWAY
+ );
assert!(
- origin.session("docs").await.is_none(),
- "removing the session is what disabling does, and a request must then 404"
+ origin.trouble.read().await["docs"].at > first,
+ "the second request did not reach the dial"
);
+ }
+
+ /// But not once per request, because a page decides how often those arrive.
+ ///
+ /// `
` in a loop is a remote document choosing how
+ /// often this machine opens an ssh. It cannot name a host that was not declared -- the set
+ /// is fixed at startup and is not `ssh_config` -- so the reach is a host the reader already
+ /// asked to have served. That makes it cheap, not free, and the rate is the part somebody
+ /// else was choosing.
+ #[tokio::test]
+ async fn a_page_cannot_choose_how_often_an_ssh_is_opened() {
+ let mut origin = origin_with(one_page()).await;
+ origin.cooldown = Duration::from_secs(300);
+ origin.sessions.write().await.remove("docs");
+
+ assert_eq!(
+ origin.handle(get("/a.html", None)).await.status(),
+ StatusCode::BAD_GATEWAY
+ );
+ let first = origin.trouble.read().await["docs"].at;
+
+ for _ in 0..5 {
+ assert_eq!(
+ origin.handle(get("/a.html", None)).await.status(),
+ StatusCode::BAD_GATEWAY,
+ "the answer is still the truth, it just costs nothing to give"
+ );
+ }
+ assert_eq!(
+ origin.trouble.read().await["docs"].at,
+ first,
+ "a loop of requests dialled more than once inside the cooldown"
+ );
+ }
+
+ /// A connection that dies is dropped, not held.
+ ///
+ /// Without this the daemon answers `sftp session is gone` for as long as it runs. It is
+ /// the one failure mode a reconnecting daemon must not have, and the only way to see it is
+ /// to kill a real connection under a live origin.
+ #[tokio::test]
+ async fn a_dead_connection_is_not_held() {
+ let (fs, stop) = one_page().spawn_stoppable().await;
+ let origin = origin_over(fs, Cache::default());
+ assert_eq!(
+ origin.handle(get("/a.html", None)).await.status(),
+ StatusCode::OK
+ );
+
+ stop.abort();
+ // The driver task notices when its reader fails, which is a scheduling hop away rather
+ // than a duration. Bounded so a change that stops it noticing fails here instead of
+ // hanging the suite.
+ for _ in 0..1000 {
+ if origin.live("docs").await.is_none() {
+ break;
+ }
+ tokio::task::yield_now().await;
+ }
+ assert!(
+ origin.live("docs").await.is_none(),
+ "a connection whose remote is gone must not stay in the map"
+ );
+
+ // And what follows is a re-dial rather than a corpse. Both are 502, so the status is
+ // not the evidence -- what it says is. `nowhere` is not a host, so a request that
+ // re-dialled reports ssh failing to reach it; one that went down the dead connection
+ // instead would report the pipe, and only the request after that would try again.
let res = origin.handle(get("/a.html", None)).await;
+ assert_eq!(res.status(), StatusCode::BAD_GATEWAY);
+ let said = String::from_utf8_lossy(&body_of(res).await).to_string();
+ assert!(said.contains("is not connected"), "{said}");
+ assert!(
+ !said.contains("session closed"),
+ "the dead connection was used rather than replaced: {said}"
+ );
+
+ // The window this cannot reach: a connection that is gone but whose driver task has
+ // not noticed yet, because the notice arrives when its own read fails and a request
+ // can be handed down the pipe before that. An in-memory duplex closes synchronously,
+ // so there is no window here to open. It was measured instead against a real host --
+ // kill the ssh under a live daemon, reload once -- three times for three reloads, all
+ // 200. Before `alias` checked liveness *after* the attempt, that reload was a 502 and
+ // only the one after it worked.
+ }
+
+ /// A name nobody declared stays a 404.
+ ///
+ /// The three answers have to stay three. If everything unreachable became 502 then a typo
+ /// in a URL would read as somebody's host being down.
+ #[tokio::test]
+ async fn an_undeclared_name_is_not_a_gateway_failure() {
+ let origin = origin_with(one_page()).await;
+ let res = origin.handle(get_on("typo", "/a.html")).await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
}
+ #[tokio::test]
+ async fn disabling_a_host_closes_it_now() {
+ let origin = origin_with(one_page()).await;
+ // A host opened from the dashboard, not a declared alias. The two are closed on
+ // different terms now and this is the one disabling is about: it was never in
+ // `declared`, so removing the session is the whole of stopping it and the name goes
+ // back to being one this daemon does not serve. Sharing the `Arc` rather than opening
+ // a second remote means dropping this one leaves `docs` connected, which is the point
+ // -- disabling one host must not disturb another.
+ let session = Arc::clone(&origin.sessions.read().await["docs"]);
+ origin
+ .sessions
+ .write()
+ .await
+ .insert("opened".to_string(), session);
+ assert!(origin.live("opened").await.is_some());
+
+ origin.sessions.write().await.remove("opened");
+ assert!(origin.live("opened").await.is_none());
+ let res = origin.handle(get_on("opened", "/a.html")).await;
+ assert_eq!(res.status(), StatusCode::NOT_FOUND);
+ assert_eq!(
+ origin.handle(get("/a.html", None)).await.status(),
+ StatusCode::OK,
+ "closing one must not disturb another"
+ );
+ }
+
/// Nothing about how to reach a host is reported to anything but the dashboard.
///
/// The point of the whole arrangement: `ssh_config` keeps the account, the port and the
@@ -4845,9 +5428,20 @@ mod tests {
// The origin stops answering, rather than answering with stale bytes out of the
// cache. An alias that is closed but still serving would be the worst of both.
+ //
+ // 503 and not 404, and the difference is the whole of this change: `docs` is still an
+ // alias this daemon serves, it has been switched off. A 404 here would say the name
+ // was never known, which is what the daemon used to say about every host that was
+ // merely asleep -- and the reader would go looking in the config file for a name that
+ // is sitting right there in it.
+ let res = origin.handle(get("/a.html", None)).await;
+ assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE);
+
+ // And a reload does not undo it. A stop that the next request reversed would not be a
+ // stop, and this is the exact path that would reverse it.
assert_eq!(
origin.handle(get("/a.html", None)).await.status(),
- StatusCode::NOT_FOUND
+ StatusCode::SERVICE_UNAVAILABLE
);
}
diff --git a/crates/ssh-browser/src/testing.rs b/crates/ssh-browser/src/testing.rs
index 6434f65..9193f73 100644
--- a/crates/ssh-browser/src/testing.rs
+++ b/crates/ssh-browser/src/testing.rs
@@ -127,13 +127,24 @@ impl FakeRemote {
/// Start serving, and hand back a client driving it through the real `SftpFs`,
/// including its writer coalescing and its round-trip counter.
pub async fn spawn(self) -> SftpFs {
+ self.spawn_stoppable().await.0
+ }
+
+ /// The same, but handing back the way to end the remote.
+ ///
+ /// Aborting it drops the server's half of the pipe, which ends the client's driver task --
+ /// which is what happens when an ssh dies. There is no other way to produce a dead
+ /// `SftpFs` to test against, and a daemon that reconnects but has never been asked to face
+ /// a dead connection is a daemon that reconnects in the comments.
+ pub async fn spawn_stoppable(self) -> (SftpFs, tokio::task::AbortHandle) {
let (client, server) = tokio::io::duplex(1 << 20);
let (cr, cw) = tokio::io::split(client);
let (sr, sw) = tokio::io::split(server);
- tokio::spawn(serve(self, sr, sw));
- SftpFs::over(cw, cr)
+ let serving = tokio::spawn(serve(self, sr, sw));
+ let fs = SftpFs::over(cw, cr)
.await
- .expect("handshake with the in-memory remote")
+ .expect("handshake with the in-memory remote");
+ (fs, serving.abort_handle())
}
}
diff --git a/extension/src/dashboard.ts b/extension/src/dashboard.ts
index c75ec62..052d792 100644
--- a/extension/src/dashboard.ts
+++ b/extension/src/dashboard.ts
@@ -15,6 +15,19 @@ interface OpenAlias {
url: string;
}
+/// A declared alias with nothing connected behind it.
+///
+/// The other half of `OpenAlias`, and the half that used to be impossible: an alias was either
+/// being served or the daemon had refused to start. Now a host can be asleep without taking
+/// the rest down, which is only an improvement if the reader is told which ones are.
+interface StalledAlias {
+ alias: string;
+ host: string;
+ url: string;
+ why?: string;
+ stopped: boolean;
+}
+
interface KnownHost {
alias: string;
host: string;
@@ -36,6 +49,7 @@ interface Reply {
detail: string;
suffix?: string;
open?: OpenAlias[];
+ stalled?: StalledAlias[];
hosts?: KnownHost[];
unusable?: { host: string; why: string }[];
url?: string;
@@ -206,6 +220,37 @@ function renderList(): void {
view.append(list);
}
+ // Between the sites and the hosts, because that is what they are: sites that are not up.
+ // Each says what to do about it, because "panza — not connected" is the same silence as not
+ // listing it at all.
+ for (const s of latest.stalled ?? []) {
+ const box = document.createElement("div");
+ box.className = "act";
+ box.append(node("h2", "", `${s.alias} is not connected`));
+ box.append(
+ node(
+ "p",
+ "bad-host",
+ s.stopped
+ ? `Stopped from here. It is in the daemon's configuration, so restarting serves it ` +
+ `again — reloading the page will not.`
+ : (s.why ?? `${s.host} did not answer.`),
+ ),
+ );
+ if (!s.stopped) {
+ const retry = document.createElement("a");
+ retry.className = "open-site";
+ retry.href = s.url;
+ retry.target = "_blank";
+ retry.rel = "noreferrer";
+ // Opening it *is* the retry. A button here that dialled behind the reader's back would
+ // be a second way to do the same thing, and the one they already know is the address.
+ retry.textContent = `Try ${s.url}`;
+ box.append(retry);
+ }
+ view.append(box);
+ }
+
const spare = hosts.filter((h) => !open.some((o) => o.alias === h.alias));
view.append(node("h2", "", "hosts you can serve"));
if (spare.length === 0) {