Context
We’ve had recurring bugs related to window UUIDs. Today, UUIDs are passed around extensively so messaging can be routed to the correct window. The issue is that code often captures a UUID and uses it asynchronously or after a suspension point or delay etc. In the meantime, iOS may have already torn down that window. This means every caller using a UUID is responsible for verifying the window is still alive before using it. In theory this is fine, but in practice it’s easy to miss this housekeeping.
When WindowManager was originally designed, APIs like tabManager(for: UUID) returned non-optionals because:
- Querying a freed window was considered a caller error
- Returning optionals requires unwrapping / guards at each call site
- Non-optionals "force the issue" of using UUIDs responsibly, since there is not really a great way of returning a non-optional in the case where an invalid UUID is passed in
Proposal
Moving forward, key WindowManager APIs will return optionals, and require callers to guard / log at the point of use. (For now the main change is tabManager(for:))
Pros: Moves responsibility more explicitly to the call site; Easier to support graceful / non-fatal failures
Cons: Could mask bugs if callers silently early-return without logging (we can still log in tabManager(for:) though if we're concerned about this); Requires extra boilerplate; Requires all code calling these APIs to have a way to fail gracefully
We've already started moving in this direction, in some areas we first check windowExists() before accessing the tabManager; returning optionals would sort of just formalize this pattern and express it in a different way.
┆Issue is synchronized with this Jira Task
Context
We’ve had recurring bugs related to window UUIDs. Today, UUIDs are passed around extensively so messaging can be routed to the correct window. The issue is that code often captures a UUID and uses it asynchronously or after a suspension point or delay etc. In the meantime, iOS may have already torn down that window. This means every caller using a UUID is responsible for verifying the window is still alive before using it. In theory this is fine, but in practice it’s easy to miss this housekeeping.
When WindowManager was originally designed, APIs like
tabManager(for: UUID)returned non-optionals because:Proposal
Moving forward, key WindowManager APIs will return optionals, and require callers to guard / log at the point of use. (For now the main change is
tabManager(for:))Pros: Moves responsibility more explicitly to the call site; Easier to support graceful / non-fatal failures
Cons: Could mask bugs if callers silently early-return without logging (we can still log in
tabManager(for:)though if we're concerned about this); Requires extra boilerplate; Requires all code calling these APIs to have a way to fail gracefullyWe've already started moving in this direction, in some areas we first check
windowExists()before accessing the tabManager; returning optionals would sort of just formalize this pattern and express it in a different way.┆Issue is synchronized with this Jira Task