Merge pull request #486 from manaflow-ai/task-sentry-0-61-0-issues
Harden Sentry crash and noise guards for 0.61.0
This commit is contained in:
commit
ca2e975a0d
4 changed files with 346 additions and 34 deletions
|
|
@ -826,12 +826,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
|
|||
|
||||
// Performance tracing (10% of transactions)
|
||||
options.tracesSampleRate = 0.1
|
||||
// App hang timeout (default is 2s, be explicit)
|
||||
options.appHangTimeoutInterval = 2.0
|
||||
// Keep app-hang tracking enabled, but avoid reporting short main-thread stalls
|
||||
// as hangs in normal user interaction flows.
|
||||
options.appHangTimeoutInterval = 8.0
|
||||
// Attach stack traces to all events
|
||||
options.attachStacktrace = true
|
||||
// Capture failed HTTP requests
|
||||
options.enableCaptureFailedRequests = true
|
||||
// Avoid recursively capturing failed requests from Sentry's own ingestion endpoint.
|
||||
options.enableCaptureFailedRequests = false
|
||||
}
|
||||
|
||||
if !isRunningUnderXCTest {
|
||||
|
|
@ -6616,9 +6617,23 @@ private var cmuxFirstResponderGuardCurrentEventOverride: NSEvent?
|
|||
private var cmuxFirstResponderGuardHitViewOverride: NSView?
|
||||
#endif
|
||||
private var cmuxBrowserReturnForwardingDepth = 0
|
||||
private var cmuxFieldEditorOwningWebViewAssociationKey: UInt8 = 0
|
||||
|
||||
private final class CmuxFieldEditorOwningWebViewBox: NSObject {
|
||||
weak var webView: CmuxWebView?
|
||||
|
||||
init(webView: CmuxWebView?) {
|
||||
self.webView = webView
|
||||
}
|
||||
}
|
||||
|
||||
private extension NSWindow {
|
||||
@objc func cmux_makeFirstResponder(_ responder: NSResponder?) -> Bool {
|
||||
let currentEvent = Self.cmuxCurrentEvent(for: self)
|
||||
let responderWebView = responder.flatMap {
|
||||
Self.cmuxOwningWebView(for: $0, in: self, event: currentEvent)
|
||||
}
|
||||
|
||||
if AppDelegate.shared?.shouldBlockFirstResponderChangeWhileCommandPaletteVisible(
|
||||
window: self,
|
||||
responder: responder
|
||||
|
|
@ -6633,9 +6648,8 @@ private extension NSWindow {
|
|||
}
|
||||
|
||||
if let responder,
|
||||
let webView = Self.cmuxOwningWebView(for: responder),
|
||||
let webView = responderWebView,
|
||||
!webView.allowsFirstResponderAcquisitionEffective {
|
||||
let currentEvent = Self.cmuxCurrentEvent(for: self)
|
||||
let pointerInitiatedFocus = Self.cmuxShouldAllowPointerInitiatedWebViewFocus(
|
||||
window: self,
|
||||
webView: webView,
|
||||
|
|
@ -6668,7 +6682,7 @@ private extension NSWindow {
|
|||
}
|
||||
#if DEBUG
|
||||
if let responder,
|
||||
let webView = Self.cmuxOwningWebView(for: responder) {
|
||||
let webView = responderWebView {
|
||||
dlog(
|
||||
"focus.guard allowFirstResponder responder=\(String(describing: type(of: responder))) " +
|
||||
"window=\(ObjectIdentifier(self)) " +
|
||||
|
|
@ -6678,7 +6692,15 @@ private extension NSWindow {
|
|||
)
|
||||
}
|
||||
#endif
|
||||
return cmux_makeFirstResponder(responder)
|
||||
let result = cmux_makeFirstResponder(responder)
|
||||
if result {
|
||||
if let fieldEditor = responder as? NSTextView, fieldEditor.isFieldEditor {
|
||||
Self.cmuxTrackFieldEditor(fieldEditor, owningWebView: responderWebView)
|
||||
} else if let fieldEditor = self.firstResponder as? NSTextView, fieldEditor.isFieldEditor {
|
||||
Self.cmuxTrackFieldEditor(fieldEditor, owningWebView: responderWebView)
|
||||
}
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
@objc func cmux_sendEvent(_ event: NSEvent) {
|
||||
|
|
@ -6733,7 +6755,9 @@ private extension NSWindow {
|
|||
// (handleCustomShortcut) already handles app-level shortcuts, and anything
|
||||
// remaining should be menu items.
|
||||
let firstResponderGhosttyView = cmuxOwningGhosttyView(for: self.firstResponder)
|
||||
let firstResponderWebView = self.firstResponder.flatMap { Self.cmuxOwningWebView(for: $0) }
|
||||
let firstResponderWebView = self.firstResponder.flatMap {
|
||||
Self.cmuxOwningWebView(for: $0, in: self, event: event)
|
||||
}
|
||||
if let ghosttyView = firstResponderGhosttyView {
|
||||
// If the IME is composing, don't intercept key events — let them flow
|
||||
// through normal AppKit event dispatch so the input method can process them.
|
||||
|
|
@ -6857,12 +6881,8 @@ private extension NSWindow {
|
|||
return webView
|
||||
}
|
||||
|
||||
if let textView = responder as? NSTextView,
|
||||
let delegateView = textView.delegate as? NSView,
|
||||
let webView = cmuxOwningWebView(for: delegateView) {
|
||||
return webView
|
||||
}
|
||||
|
||||
// NSTextView.delegate is unsafe-unretained in AppKit. Reading it here while
|
||||
// a responder chain is tearing down can trap with "unowned reference".
|
||||
var current = responder.nextResponder
|
||||
while let next = current {
|
||||
if let webView = next as? CmuxWebView {
|
||||
|
|
@ -6878,6 +6898,28 @@ private extension NSWindow {
|
|||
return nil
|
||||
}
|
||||
|
||||
private static func cmuxOwningWebView(
|
||||
for responder: NSResponder,
|
||||
in window: NSWindow,
|
||||
event: NSEvent?
|
||||
) -> CmuxWebView? {
|
||||
if let webView = cmuxOwningWebView(for: responder) {
|
||||
return webView
|
||||
}
|
||||
|
||||
guard let textView = responder as? NSTextView, textView.isFieldEditor else {
|
||||
return nil
|
||||
}
|
||||
|
||||
if let event,
|
||||
let hitWebView = cmuxPointerHitWebView(in: window, event: event) {
|
||||
cmuxTrackFieldEditor(textView, owningWebView: hitWebView)
|
||||
return hitWebView
|
||||
}
|
||||
|
||||
return cmuxTrackedOwningWebView(for: textView)
|
||||
}
|
||||
|
||||
private static func cmuxOwningWebView(for view: NSView) -> CmuxWebView? {
|
||||
if let webView = view as? CmuxWebView {
|
||||
return webView
|
||||
|
|
@ -6912,28 +6954,68 @@ private extension NSWindow {
|
|||
return window.contentView?.hitTest(event.locationInWindow)
|
||||
}
|
||||
|
||||
private static func cmuxTrackFieldEditor(_ fieldEditor: NSTextView, owningWebView webView: CmuxWebView?) {
|
||||
if let webView {
|
||||
objc_setAssociatedObject(
|
||||
fieldEditor,
|
||||
&cmuxFieldEditorOwningWebViewAssociationKey,
|
||||
CmuxFieldEditorOwningWebViewBox(webView: webView),
|
||||
.OBJC_ASSOCIATION_RETAIN_NONATOMIC
|
||||
)
|
||||
} else {
|
||||
objc_setAssociatedObject(
|
||||
fieldEditor,
|
||||
&cmuxFieldEditorOwningWebViewAssociationKey,
|
||||
nil,
|
||||
.OBJC_ASSOCIATION_RETAIN_NONATOMIC
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private static func cmuxTrackedOwningWebView(for fieldEditor: NSTextView) -> CmuxWebView? {
|
||||
guard let box = objc_getAssociatedObject(
|
||||
fieldEditor,
|
||||
&cmuxFieldEditorOwningWebViewAssociationKey
|
||||
) as? CmuxFieldEditorOwningWebViewBox else {
|
||||
return nil
|
||||
}
|
||||
guard let webView = box.webView else {
|
||||
cmuxTrackFieldEditor(fieldEditor, owningWebView: nil)
|
||||
return nil
|
||||
}
|
||||
return webView
|
||||
}
|
||||
|
||||
private static func cmuxIsPointerDownEvent(_ event: NSEvent) -> Bool {
|
||||
switch event.type {
|
||||
case .leftMouseDown, .rightMouseDown, .otherMouseDown:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
private static func cmuxPointerHitWebView(in window: NSWindow, event: NSEvent) -> CmuxWebView? {
|
||||
guard cmuxIsPointerDownEvent(event) else { return nil }
|
||||
if event.windowNumber != 0, event.windowNumber != window.windowNumber {
|
||||
return nil
|
||||
}
|
||||
if let eventWindow = event.window, eventWindow !== window {
|
||||
return nil
|
||||
}
|
||||
guard let hitView = cmuxHitViewForCurrentEvent(in: window, event: event) else {
|
||||
return nil
|
||||
}
|
||||
return cmuxOwningWebView(for: hitView)
|
||||
}
|
||||
|
||||
private static func cmuxShouldAllowPointerInitiatedWebViewFocus(
|
||||
window: NSWindow,
|
||||
webView: CmuxWebView,
|
||||
event: NSEvent?
|
||||
) -> Bool {
|
||||
guard let event else { return false }
|
||||
switch event.type {
|
||||
case .leftMouseDown, .rightMouseDown, .otherMouseDown:
|
||||
break
|
||||
default:
|
||||
return false
|
||||
}
|
||||
|
||||
if event.windowNumber != 0, event.windowNumber != window.windowNumber {
|
||||
return false
|
||||
}
|
||||
if let eventWindow = event.window, eventWindow !== window {
|
||||
return false
|
||||
}
|
||||
|
||||
guard let hitView = cmuxHitViewForCurrentEvent(in: window, event: event),
|
||||
let hitWebView = cmuxOwningWebView(for: hitView) else {
|
||||
guard let event,
|
||||
let hitWebView = cmuxPointerHitWebView(in: window, event: event) else {
|
||||
return false
|
||||
}
|
||||
return hitWebView === webView
|
||||
|
|
|
|||
|
|
@ -321,7 +321,11 @@ class GhosttyApp {
|
|||
private var scrollLagSampleCount = 0
|
||||
private var scrollLagTotalMs: Double = 0
|
||||
private var scrollLagMaxMs: Double = 0
|
||||
private let scrollLagThresholdMs: Double = 25 // Alert if tick takes >25ms during scroll
|
||||
private let scrollLagThresholdMs: Double = 40
|
||||
private let scrollLagMinimumSamples = 8
|
||||
private let scrollLagMinimumAverageMs: Double = 12
|
||||
private let scrollLagReportCooldownSeconds: TimeInterval = 300
|
||||
private var lastScrollLagReportUptime: TimeInterval?
|
||||
private var scrollEndTimer: DispatchWorkItem?
|
||||
|
||||
func markScrollActivity(hasMomentum: Bool, momentumEnded: Bool) {
|
||||
|
|
@ -356,7 +360,18 @@ class GhosttyApp {
|
|||
let maxLag = scrollLagMaxMs
|
||||
let samples = scrollLagSampleCount
|
||||
let threshold = scrollLagThresholdMs
|
||||
if maxLag > threshold {
|
||||
let nowUptime = ProcessInfo.processInfo.systemUptime
|
||||
if Self.shouldCaptureScrollLagEvent(
|
||||
samples: samples,
|
||||
averageMs: avgLag,
|
||||
maxMs: maxLag,
|
||||
thresholdMs: threshold,
|
||||
minimumSamples: scrollLagMinimumSamples,
|
||||
minimumAverageMs: scrollLagMinimumAverageMs,
|
||||
nowUptime: nowUptime,
|
||||
lastReportedUptime: lastScrollLagReportUptime,
|
||||
cooldown: scrollLagReportCooldownSeconds
|
||||
) {
|
||||
SentrySDK.capture(message: "Scroll lag detected") { scope in
|
||||
scope.setLevel(.warning)
|
||||
scope.setContext(value: [
|
||||
|
|
@ -366,6 +381,7 @@ class GhosttyApp {
|
|||
"threshold_ms": threshold
|
||||
], key: "scroll_lag")
|
||||
}
|
||||
lastScrollLagReportUptime = nowUptime
|
||||
}
|
||||
// Reset stats
|
||||
scrollLagSampleCount = 0
|
||||
|
|
@ -624,6 +640,29 @@ class GhosttyApp {
|
|||
previousColorScheme != currentColorScheme
|
||||
}
|
||||
|
||||
static func shouldCaptureScrollLagEvent(
|
||||
samples: Int,
|
||||
averageMs: Double,
|
||||
maxMs: Double,
|
||||
thresholdMs: Double,
|
||||
minimumSamples: Int = 8,
|
||||
minimumAverageMs: Double = 12,
|
||||
nowUptime: TimeInterval,
|
||||
lastReportedUptime: TimeInterval?,
|
||||
cooldown: TimeInterval = 300
|
||||
) -> Bool {
|
||||
guard samples >= minimumSamples else { return false }
|
||||
guard averageMs.isFinite, maxMs.isFinite, thresholdMs.isFinite, nowUptime.isFinite, cooldown.isFinite else {
|
||||
return false
|
||||
}
|
||||
guard averageMs >= minimumAverageMs else { return false }
|
||||
guard maxMs > thresholdMs else { return false }
|
||||
if let lastReportedUptime, nowUptime - lastReportedUptime < cooldown {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
private func loadLegacyGhosttyConfigIfNeeded(_ config: ghostty_config_t) {
|
||||
#if os(macOS)
|
||||
// Ghostty 1.3+ prefers `config.ghostty`, but some users still have their real
|
||||
|
|
|
|||
|
|
@ -113,6 +113,39 @@ final class CmuxWebViewKeyEquivalentTests: XCTestCase {
|
|||
override var acceptsFirstResponder: Bool { true }
|
||||
}
|
||||
|
||||
private final class DelegateProbeTextView: NSTextView {
|
||||
private(set) var delegateReadCount = 0
|
||||
|
||||
override var delegate: NSTextViewDelegate? {
|
||||
get {
|
||||
delegateReadCount += 1
|
||||
return super.delegate
|
||||
}
|
||||
set {
|
||||
super.delegate = newValue
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private final class FieldEditorProbeTextView: NSTextView {
|
||||
private(set) var delegateReadCount = 0
|
||||
|
||||
override var delegate: NSTextViewDelegate? {
|
||||
get {
|
||||
delegateReadCount += 1
|
||||
return super.delegate
|
||||
}
|
||||
set {
|
||||
super.delegate = newValue
|
||||
}
|
||||
}
|
||||
|
||||
override var isFieldEditor: Bool {
|
||||
get { true }
|
||||
set {}
|
||||
}
|
||||
}
|
||||
|
||||
func testCmdNRoutesToMainMenuWhenWebViewIsFirstResponder() {
|
||||
let spy = ActionSpy()
|
||||
installMenu(spy: spy, key: "n", modifiers: [.command])
|
||||
|
|
@ -377,6 +410,96 @@ final class CmuxWebViewKeyEquivalentTests: XCTestCase {
|
|||
XCTAssertFalse(window.makeFirstResponder(descendant), "Expected pointer bypass to be limited to click context")
|
||||
}
|
||||
|
||||
@MainActor
|
||||
func testWindowFirstResponderGuardAvoidsTextViewDelegateLookupForWebViewResolution() {
|
||||
_ = NSApplication.shared
|
||||
AppDelegate.installWindowResponderSwizzlesForTesting()
|
||||
|
||||
let window = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 640, height: 420),
|
||||
styleMask: [.titled, .closable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
let container = NSView(frame: window.contentRect(forFrameRect: window.frame))
|
||||
window.contentView = container
|
||||
|
||||
let textView = DelegateProbeTextView(frame: NSRect(x: 0, y: 0, width: 100, height: 40))
|
||||
container.addSubview(textView)
|
||||
|
||||
window.makeKeyAndOrderFront(nil)
|
||||
defer { window.orderOut(nil) }
|
||||
|
||||
_ = window.makeFirstResponder(nil)
|
||||
_ = window.makeFirstResponder(textView)
|
||||
|
||||
XCTAssertEqual(
|
||||
textView.delegateReadCount,
|
||||
0,
|
||||
"WebView ownership resolution should not touch NSTextView.delegate (unsafe-unretained in AppKit)"
|
||||
)
|
||||
}
|
||||
|
||||
@MainActor
|
||||
func testWindowFirstResponderGuardResolvesTrackedWebViewForFieldEditorResponder() {
|
||||
_ = NSApplication.shared
|
||||
AppDelegate.installWindowResponderSwizzlesForTesting()
|
||||
|
||||
let window = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 640, height: 420),
|
||||
styleMask: [.titled, .closable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
let container = NSView(frame: window.contentRect(forFrameRect: window.frame))
|
||||
window.contentView = container
|
||||
|
||||
let webView = CmuxWebView(frame: container.bounds, configuration: WKWebViewConfiguration())
|
||||
webView.autoresizingMask = [.width, .height]
|
||||
container.addSubview(webView)
|
||||
|
||||
let descendant = FirstResponderView(frame: NSRect(x: 0, y: 0, width: 10, height: 10))
|
||||
webView.addSubview(descendant)
|
||||
|
||||
let fieldEditor = FieldEditorProbeTextView(frame: NSRect(x: 0, y: 0, width: 100, height: 20))
|
||||
|
||||
window.makeKeyAndOrderFront(nil)
|
||||
defer {
|
||||
AppDelegate.clearWindowFirstResponderGuardTesting()
|
||||
window.orderOut(nil)
|
||||
}
|
||||
|
||||
webView.allowsFirstResponderAcquisition = true
|
||||
XCTAssertTrue(window.makeFirstResponder(descendant))
|
||||
|
||||
let timestamp = ProcessInfo.processInfo.systemUptime
|
||||
let pointerDownEvent = NSEvent.mouseEvent(
|
||||
with: .leftMouseDown,
|
||||
location: NSPoint(x: 5, y: 5),
|
||||
modifierFlags: [],
|
||||
timestamp: timestamp,
|
||||
windowNumber: window.windowNumber,
|
||||
context: nil,
|
||||
eventNumber: 1,
|
||||
clickCount: 1,
|
||||
pressure: 1.0
|
||||
)
|
||||
XCTAssertNotNil(pointerDownEvent)
|
||||
|
||||
AppDelegate.setWindowFirstResponderGuardTesting(currentEvent: pointerDownEvent, hitView: descendant)
|
||||
XCTAssertTrue(window.makeFirstResponder(fieldEditor))
|
||||
|
||||
AppDelegate.clearWindowFirstResponderGuardTesting()
|
||||
_ = window.makeFirstResponder(nil)
|
||||
webView.allowsFirstResponderAcquisition = false
|
||||
XCTAssertFalse(window.makeFirstResponder(fieldEditor))
|
||||
XCTAssertEqual(
|
||||
fieldEditor.delegateReadCount,
|
||||
0,
|
||||
"Field-editor webview ownership should come from tracked associations, not NSTextView.delegate"
|
||||
)
|
||||
}
|
||||
|
||||
private func installMenu(spy: ActionSpy, key: String, modifiers: NSEvent.ModifierFlags) {
|
||||
let mainMenu = NSMenu()
|
||||
|
||||
|
|
|
|||
|
|
@ -225,6 +225,74 @@ final class GhosttyConfigTests: XCTestCase {
|
|||
)
|
||||
}
|
||||
|
||||
func testScrollLagCaptureRequiresSustainedLag() {
|
||||
XCTAssertFalse(
|
||||
GhosttyApp.shouldCaptureScrollLagEvent(
|
||||
samples: 4,
|
||||
averageMs: 18,
|
||||
maxMs: 85,
|
||||
thresholdMs: 40,
|
||||
nowUptime: 1000,
|
||||
lastReportedUptime: nil
|
||||
)
|
||||
)
|
||||
XCTAssertFalse(
|
||||
GhosttyApp.shouldCaptureScrollLagEvent(
|
||||
samples: 10,
|
||||
averageMs: 6,
|
||||
maxMs: 85,
|
||||
thresholdMs: 40,
|
||||
nowUptime: 1000,
|
||||
lastReportedUptime: nil
|
||||
)
|
||||
)
|
||||
XCTAssertFalse(
|
||||
GhosttyApp.shouldCaptureScrollLagEvent(
|
||||
samples: 10,
|
||||
averageMs: 18,
|
||||
maxMs: 35,
|
||||
thresholdMs: 40,
|
||||
nowUptime: 1000,
|
||||
lastReportedUptime: nil
|
||||
)
|
||||
)
|
||||
XCTAssertTrue(
|
||||
GhosttyApp.shouldCaptureScrollLagEvent(
|
||||
samples: 10,
|
||||
averageMs: 18,
|
||||
maxMs: 85,
|
||||
thresholdMs: 40,
|
||||
nowUptime: 1000,
|
||||
lastReportedUptime: nil
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testScrollLagCaptureRespectsCooldownWindow() {
|
||||
XCTAssertFalse(
|
||||
GhosttyApp.shouldCaptureScrollLagEvent(
|
||||
samples: 12,
|
||||
averageMs: 22,
|
||||
maxMs: 90,
|
||||
thresholdMs: 40,
|
||||
nowUptime: 1200,
|
||||
lastReportedUptime: 1005,
|
||||
cooldown: 300
|
||||
)
|
||||
)
|
||||
XCTAssertTrue(
|
||||
GhosttyApp.shouldCaptureScrollLagEvent(
|
||||
samples: 12,
|
||||
averageMs: 22,
|
||||
maxMs: 90,
|
||||
thresholdMs: 40,
|
||||
nowUptime: 1406,
|
||||
lastReportedUptime: 1005,
|
||||
cooldown: 300
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testClaudeCodeIntegrationDefaultsToEnabledWhenUnset() {
|
||||
let suiteName = "cmux.tests.claude-hooks.\(UUID().uuidString)"
|
||||
guard let defaults = UserDefaults(suiteName: suiteName) else {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue