Skip to content

Commit

Permalink
Do not hard-crash on missing usernames (#448)
Browse files Browse the repository at this point in the history
Previously we were using `username!` to look up usernames. This would
throw a fatal exception if the username was not available, crashing the
app. In particular attempts to update HealthKit metrics could hit this
after logging out from the app.

Avoid this by centralizing the username lookup logic in the request
manager making URLs in the app slightly more readable, and have it throw
a catchable exception. This could theoretically cause issues in the case
that a URL contains an additional `{username}` string, but this seems
unlikely.

Testing:
Launched the app on my phone and checked it could load goals.
Signed out and did not immediately experience a crash.
  • Loading branch information
theospears authored Feb 29, 2024
1 parent 8c657b8 commit b5a3e79
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 25 deletions.
10 changes: 5 additions & 5 deletions BeeKit/Goal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ public class Goal {
Task { @MainActor in
let params = ["sort" : "daystamp", "count" : 7] as [String : Any]
do {
let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints.json", parameters: params)
let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints.json", parameters: params)
let responseJSON = JSON(response!)
success(try ExistingDataPoint.fromJSONArray(array: responseJSON.arrayValue))
} catch {
Expand All @@ -377,7 +377,7 @@ public class Goal {
"comment": "Auto-updated via Apple Health",
]
do {
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints/\(datapoint.id).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints/\(datapoint.id).json", parameters: params)
success?()
} catch {
errorCompletion?()
Expand All @@ -388,7 +388,7 @@ public class Goal {
func deleteDatapoint(datapoint : ExistingDataPoint) {
Task { @MainActor in
do {
let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints/\(datapoint.id)", parameters: nil)
let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints/\(datapoint.id)", parameters: nil)
} catch {
logger.error("Error deleting datapoint: \(error)")
}
Expand All @@ -398,7 +398,7 @@ public class Goal {
func postDatapoint(params : [String : String], success : ((Any?) -> Void)?, failure : ((Error?, String?) -> Void)?) {
Task { @MainActor in
do {
let response = try await ServiceLocator.requestManager.post(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints.json", parameters: params)
let response = try await ServiceLocator.requestManager.post(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints.json", parameters: params)
success?(response)
} catch {
failure?(error, error.localizedDescription)
Expand All @@ -408,7 +408,7 @@ public class Goal {

func fetchDatapoints(sort: String, per: Int, page: Int) async throws -> [ExistingDataPoint] {
let params = ["sort" : sort, "per" : per, "page": page] as [String : Any]
let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints.json", parameters: params)
let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints.json", parameters: params)
let responseJSON = JSON(response!)
return try ExistingDataPoint.fromJSONArray(array: responseJSON.arrayValue)
}
Expand Down
8 changes: 6 additions & 2 deletions BeeKit/Managers/CurrentUserManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public class CurrentUserManager {
// Fetch a user from the persistent store
let request = NSFetchRequest<User>(entityName: "User")
// TODO: Handle (or at least log) an error here
let users = try? (context ?? container.viewContext).fetch(request)
let users = try? (context ?? container.newBackgroundContext()).fetch(request)
return users?.first
}

Expand Down Expand Up @@ -153,7 +153,7 @@ public class CurrentUserManager {
public var username :String? {
return user()?.username
}

public func defaultLeadTime() -> NSNumber {
return (user()?.defaultLeadTime ?? 0) as NSNumber
}
Expand Down Expand Up @@ -281,3 +281,7 @@ public class CurrentUserManager {
}.value
}
}

public enum CurrentUserManagerError : Error {
case loggedOut
}
19 changes: 14 additions & 5 deletions BeeKit/Managers/RequestManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import OSLog

struct ServerError: Error {
let message: String
let requestError: Error
let requestError: Error?

init(_ message: String, requestError: Error) {
init(_ message: String, requestError: Error?) {
self.message = message
self.requestError = requestError
}
Expand All @@ -30,9 +30,18 @@ public class RequestManager {
private let logger = Logger(subsystem: "com.beeminder.beeminder", category: "RequestManager")

func rawRequest(url: String, method: HTTPMethod, parameters: [String: Any]?, headers: HTTPHeaders) async throws -> Any? {
let encoding: ParameterEncoding = if method == .get { URLEncoding.default } else { JSONEncoding.default }

var urlWithSubstitutions = url
if url.contains("{username}") {
guard let username = ServiceLocator.currentUserManager.username else {
throw ServerError("Attempted to make request to username-based URL \(url) while logged out", requestError: nil)
}
urlWithSubstitutions = urlWithSubstitutions.replacingOccurrences(of: "{username}", with: username)
}

let encoding: ParameterEncoding = if method == .get { URLEncoding.default } else { JSONEncoding.default }// TODO
let response = await AF.request(
"\(baseURLString)/\(url)",
"\(baseURLString)/\(urlWithSubstitutions)",
method: method,
parameters: parameters,
encoding: encoding,
Expand Down Expand Up @@ -99,7 +108,7 @@ public class RequestManager {
public func addDatapoint(urtext: String, slug: String) async throws -> Any? {
let params = ["urtext": urtext, "requestid": UUID().uuidString]

return try await post(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(slug)/datapoints.json", parameters: params)
return try await post(url: "api/v1/users/{username}/goals/\(slug)/datapoints.json", parameters: params)
}
}

Expand Down
4 changes: 2 additions & 2 deletions BeeSwift/EditDatapointViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class EditDatapointViewController: UIViewController, UITextFieldDelegate {
let params = [
"urtext": self.urtext()
]
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: params)
try await ServiceLocator.goalManager.refreshGoal(self.goal)

hud.mode = .customView
Expand All @@ -234,7 +234,7 @@ class EditDatapointViewController: UIViewController, UITextFieldDelegate {
hud.mode = .indeterminate

do {
let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: nil)
let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/{username}/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: nil)
try await ServiceLocator.goalManager.refreshGoal(self.goal)

hud.mode = .customView
Expand Down
2 changes: 1 addition & 1 deletion BeeSwift/Settings/ConfigureHKMetricViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class ConfigureHKMetricViewController : UIViewController {
params = ["ii_params" : ["name" : "apple", "metric" : self.goal.healthKitMetric!]]

do {
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal.slug).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal.slug).json", parameters: params)
hud.mode = .customView
hud.customView = UIImageView(image: UIImage(named: "BasicCheckmark"))
hud.hide(animated: true, afterDelay: 2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class EditDefaultNotificationsViewController: EditNotificationsViewController {
guard let leadtime = userInfo["leadtime"] else { return }
let params = [ "default_leadtime" : leadtime ]
do {
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}.json", parameters: params)
ServiceLocator.currentUserManager.setDefaultLeadTime(leadtime)
} catch {
logger.error("Error setting default leadtime: \(error)")
Expand All @@ -52,7 +52,7 @@ class EditDefaultNotificationsViewController: EditNotificationsViewController {
self.updateAlertstartLabel(self.midnightOffsetFromTimePickerView())
let params = ["default_alertstart" : self.midnightOffsetFromTimePickerView()]
do {
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}.json", parameters: params)
ServiceLocator.currentUserManager.setDefaultAlertstart(self.midnightOffsetFromTimePickerView())
} catch {
logger.error("Error setting default alert start: \(error)")
Expand All @@ -63,7 +63,7 @@ class EditDefaultNotificationsViewController: EditNotificationsViewController {
self.updateDeadlineLabel(self.midnightOffsetFromTimePickerView())
let params = ["default_deadline" : self.midnightOffsetFromTimePickerView()]
do {
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}.json", parameters: params)
ServiceLocator.currentUserManager.setDefaultDeadline(self.midnightOffsetFromTimePickerView())
} catch {
logger.error("Error setting default deadline: \(error)")
Expand Down
10 changes: 5 additions & 5 deletions BeeSwift/Settings/EditGoalNotificationsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController {
let leadtime = userInfo["leadtime"]
let params = [ "leadtime" : leadtime, "use_defaults" : false ]
do {
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params as [String : Any])
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params as [String : Any])
self.goal!.leadtime = leadtime!
self.goal!.use_defaults = NSNumber(value: false as Bool)
self.useDefaultsSwitch.isOn = false
Expand All @@ -85,7 +85,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController {
self.updateAlertstartLabel(self.midnightOffsetFromTimePickerView())
do {
let params = ["alertstart" : self.midnightOffsetFromTimePickerView(), "use_defaults" : false]
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params)
self.goal!.alertstart = self.midnightOffsetFromTimePickerView()
self.goal!.use_defaults = NSNumber(value: false as Bool)
self.useDefaultsSwitch.isOn = false
Expand All @@ -98,7 +98,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController {
self.updateDeadlineLabel(self.midnightOffsetFromTimePickerView())
do {
let params = ["deadline" : self.midnightOffsetFromTimePickerView(), "use_defaults" : false]
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params)
self.goal?.deadline = self.midnightOffsetFromTimePickerView()
self.goal!.use_defaults = NSNumber(value: false as Bool)
self.useDefaultsSwitch.isOn = false
Expand All @@ -120,7 +120,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController {
Task { @MainActor in
do {
let params = ["use_defaults" : true]
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params)
self.goal?.use_defaults = NSNumber(value: true as Bool)
} catch {
self.logger.error("Error setting goal to use defaults: \(error)")
Expand Down Expand Up @@ -155,7 +155,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController {
Task { @MainActor in
do {
let params = ["use_defaults" : false]
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params)
let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params)
self.goal?.use_defaults = NSNumber(value: false as Bool)
} catch {
logger.error("Error setting goal to NOT use defaults: \(error)")
Expand Down
2 changes: 1 addition & 1 deletion BeeSwift/Settings/RemoveHKMetricViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class RemoveHKMetricViewController: UIViewController {

Task { @MainActor in
do {
let responseObject = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params)
let responseObject = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params)

self.goal = Goal(json: JSON(responseObject!))

Expand Down
2 changes: 1 addition & 1 deletion BeeSwift/TimerViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class TimerViewController: UIViewController {
Task { @MainActor in
do {
let params = ["urtext": self.urtext(), "requestid": UUID().uuidString]
let _ = try await ServiceLocator.requestManager.post(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug!)/datapoints.json", parameters: params)
let _ = try await ServiceLocator.requestManager.post(url: "api/v1/users/{username}/goals/\(self.slug!)/datapoints.json", parameters: params)
hud.mode = .text
hud.label.text = "Added!"
DispatchQueue.main.asyncAfter(deadline: .now() + 2, execute: {
Expand Down

0 comments on commit b5a3e79

Please sign in to comment.