Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
410 changes: 201 additions & 209 deletions apps/rustgis-web/src/web.rs

Large diffs are not rendered by default.

424 changes: 334 additions & 90 deletions crates/data/src/diagnostics.rs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions crates/engine/src/cmd/zarr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ fn fetch(root: &str, keys: &[String]) -> Result<(HashMap<String, Vec<u8>>, usize
break;
}
// A missing file is an expected answer here (see `is_absent`), not a failure: its 404
// or 403 makes no network record of its own (one note per server says the
// or 403 isn't recorded one by one (one note per server says the
// omission, and a store refusing everything still says "access denied"), while
// a real one (a 500, a timeout) does, with its address, status and duration.
let got = crate::render::rustgis_tiles::cache::download_blocking_expecting(&format!("{root}/{k}"), &[404, 403]);
let got = crate::render::rustgis_tiles::cache::download_blocking_omitting(&format!("{root}/{k}"), &[404, 403]);
match got {
Ok(b) => {
let len = b.len() as u64;
Expand Down
24 changes: 17 additions & 7 deletions crates/engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1092,9 +1092,9 @@ impl Session {
}
// A refused command is said once, without secrets, as the front ends show it.
if let Err(error) = &res {
let text = rustgis_data::diagnostics::redact(error);
if !self.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text) {
self.notify(text, true);
let text = rustgis_data::diagnostics::Redacted::new(error);
if !self.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text.as_str()) {
self.notify_redacted(text, true);
}
}
res
Expand Down Expand Up @@ -1501,10 +1501,20 @@ impl Session {
fn notice(&mut self, text: String, severity: rustgis_data::diagnostics::Severity) {
// Redacted for the status bar and the history alike. `redact` bounds the notice to 8 KiB
// (an ellipsis marks the cut) and omits one over 16 KiB whole, so a clipped credential
// can't survive; the history's copy is bounded further, to a record's 2 KiB, in `push`.
let text = rustgis_data::diagnostics::redact(&text);
self.diagnostics.push(severity, self.diagnostic_source.unwrap_or("Application"), &text, "");
self.notices.push(Notice { text, error: severity == rustgis_data::diagnostics::Severity::Error });
// can't survive; the history's copy is bounded further, to a record's 2 KiB.
self.notice_redacted(rustgis_data::diagnostics::Redacted::new(&text), severity);
}

/// [`Session::notify`] for text already redacted (a failure the caller redacted to compare it
/// with the notices): the type says so, so it isn't redacted again.
pub fn notify_redacted(&mut self, text: rustgis_data::diagnostics::Redacted, error: bool) {
use rustgis_data::diagnostics::Severity;
self.notice_redacted(text, if error { Severity::Error } else { Severity::Info });
}

fn notice_redacted(&mut self, text: rustgis_data::diagnostics::Redacted, severity: rustgis_data::diagnostics::Severity) {
self.diagnostics.push_redacted(severity, self.diagnostic_source.unwrap_or("Application"), &text);
self.notices.push(Notice { text: text.into_string(), error: severity == rustgis_data::diagnostics::Severity::Error });
self.notices_given += 1;
if self.notices.len() > 200 {
self.notices.remove(0);
Expand Down
7 changes: 1 addition & 6 deletions crates/engine/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11496,7 +11496,7 @@ fn notices_since_survives_the_cap() {
}

#[test]
fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() {
fn diagnostics_capture_notices_without_an_open_pane_and_survive_a_new_project() {
use rustgis_data::diagnostics::{Filter, MAX_DETAILS_BYTES, MAX_MESSAGE_BYTES, Severity};
let mut s = Session::new(Project::default());
let mark = s.notice_mark();
Expand All @@ -11511,8 +11511,6 @@ fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() {
assert_eq!(records.iter().map(|r| r.severity).collect::<Vec<_>>(), [Severity::Info, Severity::Warning, Severity::Error]);
assert!(records.iter().all(|r| r.source == "Application" && r.timestamp_ms > 0));
assert!(!s.diagnostics.report(Filter::All).contains("never-retain-this"));
let saved = serde_json::to_string(&s.project).unwrap();
assert!(!saved.contains("diagnostics-session-only"));
s.notify("界".repeat(3000), false);
// The notice keeps its words to redaction's own bound (8 KiB, an ellipsis at the cut), not
// the record's 2 KiB; only the history's record copy takes the message bound.
Expand All @@ -11525,9 +11523,6 @@ fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() {
let records = s.diagnostics.records();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change removed the two assertions that were the only check that diagnostics stay out of a saved project: the serde_json::to_string(&s.project) check and the reopened-session check. docs/user-guide/index.md and docs/privacy.md still say "Nothing is saved in .rgis projects", so that promise now has no test. The new test name ("survive a new project") covers the in-session part only. Restoring the removed lines, before s.run("project.new", …), would keep the guard. Confidence: medium. I did not find another test that covers this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 2b85cbc: the saved-project assert is back, guarding the docs' promise; the reopened-session assert stays removed, since Session::new always starts a fresh history, so it couldn't fail.

assert!(records.iter().any(|r| r.message == "diagnostics-session-only information"));
assert!(records.iter().any(|r| r.message == "diagnostics-session-only warning"));
// A separate session restoring the saved project must not inherit troubleshooting history.
let reopened = Session::new(serde_json::from_str(&saved).unwrap());
assert!(!reopened.diagnostics.records().iter().any(|r| r.message.starts_with("diagnostics-session-only")));
}

#[test]
Expand Down
Loading
Loading