b32f9b4 · parseable-macos

git.iko.soy/parseable-macos

on master help sign in

Fix 3 bugs: stale stream/filter loads after disconnect, query loading indicator wedge

b32f9b4iko4 months ago

ParseableViewer/App/AppState.swift+23−2

compare with master

unifiedsplit

@@ -251,10 +251,23 @@ final class AppState {

251 @MainActor

252 func refreshStreams() async {

253 guard let client else { return }

254+ // Capture the connection this fetch belongs to. A disconnect during the

255+ // await would otherwise let the (now-stale) result overwrite the cleanup

256+ // disconnect() just performed (streams = [], client = nil, etc.), leaving

257+ // the disconnected UI showing the old server's streams and re-enabling

258+ // stream-scoped tabs. Mirrors the connID guard used in performConnect,

259+ // AlertsView.loadAlerts, UsersView.loadUsers, StreamDetailView.loadData,

260+ // and ServerInfoView.loadInfo.

261+ let connID = activeConnection?.id

254 isLoadingStreams = true

262 isLoadingStreams = true

255 streamLoadError = nil

263 streamLoadError = nil

256 do {

264 do {

257− streams = try await client.listStreams()

265+ let loaded = try await client.listStreams()

266+ // Drop results if the active connection changed while awaiting. Still

267+ // clear isLoadingStreams: this task set the spinner, so leaving it true

268+ // wedges the indicator and disables Refresh.

269+ guard connID == activeConnection?.id else { isLoadingStreams = false; return }

270+ streams = loaded

258 // Drop a selection that no longer exists server-side (e.g. the stream

271 // Drop a selection that no longer exists server-side (e.g. the stream

259 // was deleted from another client); otherwise the detail tabs keep

272 // was deleted from another client); otherwise the detail tabs keep

260 // querying a missing stream and surface 404s.

273 // querying a missing stream and surface 404s.

@@ -262,6 +275,7 @@ final class AppState {

262 selectedStream = nil

275 selectedStream = nil

263 }

276 }

264 } catch {

277 } catch {

278+ guard connID == activeConnection?.id else { isLoadingStreams = false; return }

265 let message = ParseableError.userFriendlyMessage(for: error)

279 let message = ParseableError.userFriendlyMessage(for: error)

266 streamLoadError = message

280 streamLoadError = message

267 self.errorMessage = "Failed to load streams: \(message)"

281 self.errorMessage = "Failed to load streams: \(message)"

@@ -356,8 +370,15 @@ final class AppState {

356 @MainActor

370 @MainActor

357 func refreshFilters() async {

371 func refreshFilters() async {

358 guard let client else { return }

372 guard let client else { return }

373+ // Capture the connection this fetch belongs to. A disconnect during the

374+ // await would otherwise let the (now-stale) result overwrite the cleanup

375+ // disconnect() performed, resurrecting the old server's filters in the

376+ // disconnected sidebar. Same guard pattern as refreshStreams.

377+ let connID = activeConnection?.id

359 isLoadingFilters = true

378 isLoadingFilters = true

360− filters = (try? await client.listFilters()) ?? []

379+ let loaded = (try? await client.listFilters()) ?? []

380+ guard connID == activeConnection?.id else { isLoadingFilters = false; return }

381+ filters = loaded

361 isLoadingFilters = false

382 isLoadingFilters = false

362 }

383 }

363

384

ParseableViewer/ViewModels/QueryViewModel.swift+24−2

@@ -83,6 +83,14 @@ final class QueryViewModel {

84 // Query task for cancellation

85 private var queryTask: Task<Void, Never>?

86+ /// Bumped at the start of every `executeQuery`. The task captures this value

87+ /// and only clears `isLoading` on exit if it's still the active query — so a

88+ /// superseded query's late cleanup doesn't drop the loading indicator that

89+ /// the newer query set. Without this, `isLoading = false` had to live in

90+ /// every caller of `cancel()` (cancelQuery, clearResults, superseding

91+ /// executeQuery) and a future cancel path that forgot to set it would wedge

92+ /// the spinner indefinitely.

93+ private var queryGeneration = 0

87 // Time range

95 // Time range

88 var timeRangeOption: TimeRangeOption = .last1Hour

96 var timeRangeOption: TimeRangeOption = .last1Hour

@@ -401,6 +409,12 @@ final class QueryViewModel {

401

409

402 // Cancel any in-flight query

410 // Cancel any in-flight query

403 queryTask?.cancel()

411 queryTask?.cancel()

412+ // Bump the generation so the in-flight task's cleanup can detect that

413+ // it has been superseded and skip its `isLoading = false` (which would

414+ // otherwise race with the `isLoading = true` below and briefly drop

415+ // the loading indicator during this query).

416+ queryGeneration &+= 1

417+ let generation = queryGeneration

404

418

405 isLoading = true

419 isLoading = true

406 errorMessage = nil

420 errorMessage = nil

@@ -409,6 +423,16 @@ final class QueryViewModel {

409 let startTime = CFAbsoluteTimeGetCurrent()

423 let startTime = CFAbsoluteTimeGetCurrent()

410

424

411 let task = Task {

425 let task = Task {

426+ // Always clear the spinner on exit when this is still the active

427+ // query. The defer runs on every path (success, error, and the

428+ // cancellation cases that previously returned before reaching the

429+ // trailing `isLoading = false`), so the spinner can never wedge

430+ // even if a new cancel path is added without resetting it.

431+ defer {

432+ if generation == queryGeneration {

433+ isLoading = false

434+ }

435+ }

412 do {

436 do {

413 try Task.checkCancellation()

437 try Task.checkCancellation()

414 let queryResults = try await client.query(sql: sql, startTime: startDate, endTime: endDate)

438 let queryResults = try await client.query(sql: sql, startTime: startDate, endTime: endDate)

@@ -477,8 +501,6 @@ final class QueryViewModel {

477 hiddenColumns = []

501 hiddenColumns = []

478 autoHiddenColumns = []

502 autoHiddenColumns = []

479 }

503 }

480−

481− isLoading = false

482 }

504 }

483 queryTask = task

505 queryTask = task

484 await task.value

506 await task.value

ParseableViewer/Views/LiveTailView.swift+5−5

@@ -303,11 +303,6 @@ struct LiveTailView: View {

303 }

304 }

305

306− /// Computes widths for any visible columns that don't have one yet. Covers

307− /// the case where switching to a stream with an identical column set doesn't

308− /// fire `onChange(of: visibleColumns)`, which would otherwise leave widths

309− /// empty (falling back to the default width) after `selectedStream` cleared

310− /// them.

311 /// Starts (or restarts) the streaming-status pulse. Resetting `pulseOpacity`

306 /// Starts (or restarts) the streaming-status pulse. Resetting `pulseOpacity`

312 /// to 1.0 before animating to 0.3 guarantees a value delta so the repeating

307 /// to 1.0 before animating to 0.3 guarantees a value delta so the repeating

313 /// animation re-triggers even when called after a pause/resume cycle.

308 /// animation re-triggers even when called after a pause/resume cycle.

@@ -318,6 +313,11 @@ struct LiveTailView: View {

318 }

313 }

319 }

314 }

320

315

316+ /// Computes widths for any visible columns that don't have one yet. Covers

317+ /// the case where switching to a stream with an identical column set doesn't

318+ /// fire `onChange(of: visibleColumns)`, which would otherwise leave widths

319+ /// empty (falling back to the default width) after `selectedStream` cleared

320+ /// them.

321 private func ensureColumnWidths() {

322 let records = viewModel.cachedFilteredRecords

323 for col in viewModel.visibleColumns where columnWidths[col] == nil {