From f0ffdbc50c4f32e705d35b3c737cad6337e51a10 Mon Sep 17 00:00:00 2001 From: pizzaboxer Date: Tue, 3 Sep 2024 20:05:15 +0100 Subject: [PATCH] Rework error handling for HTTP API deserialization --- Bloxstrap/App.xaml.cs | 12 +++++---- .../InvalidHTTPResponseException.cs | 13 ++++++++++ Bloxstrap/Integrations/ActivityWatcher.cs | 22 +++++++++------- Bloxstrap/Integrations/DiscordRichPresence.cs | 24 ++++++++++------- Bloxstrap/JsonManager.cs | 2 +- Bloxstrap/Models/UniverseDetails.cs | 16 ++++++------ Bloxstrap/Resources/Strings.Designer.cs | 19 ++++---------- Bloxstrap/Resources/Strings.resx | 11 +++----- .../Elements/ContextMenu/ServerHistory.xaml | 22 +++++++++++++--- Bloxstrap/UI/NotifyIconWrapper.cs | 4 +++ .../ContextMenu/ServerHistoryViewModel.cs | 26 ++++++++++++++++++- Bloxstrap/Utilities.cs | 8 ------ Bloxstrap/Utility/Http.cs | 26 +++++++++---------- 13 files changed, 125 insertions(+), 80 deletions(-) create mode 100644 Bloxstrap/Exceptions/InvalidHTTPResponseException.cs diff --git a/Bloxstrap/App.xaml.cs b/Bloxstrap/App.xaml.cs index 7bfb8dfd..858b85e3 100644 --- a/Bloxstrap/App.xaml.cs +++ b/Bloxstrap/App.xaml.cs @@ -110,22 +110,24 @@ public static void FinalizeExceptionHandling(Exception ex, bool log = true) public static async Task GetLatestRelease() { const string LOG_IDENT = "App::GetLatestRelease"; - - GithubRelease? releaseInfo = null; - try { - releaseInfo = await Http.GetJson($"https://api.github.com/repos/{ProjectRepository}/releases/latest"); + var releaseInfo = await Http.GetJson($"https://api.github.com/repos/{ProjectRepository}/releases/latest"); if (releaseInfo is null || releaseInfo.Assets is null) + { Logger.WriteLine(LOG_IDENT, "Encountered invalid data"); + return null; + } + + return releaseInfo; } catch (Exception ex) { Logger.WriteException(LOG_IDENT, ex); } - return releaseInfo; + return null; } protected override void OnStartup(StartupEventArgs e) diff --git a/Bloxstrap/Exceptions/InvalidHTTPResponseException.cs b/Bloxstrap/Exceptions/InvalidHTTPResponseException.cs new file mode 100644 index 00000000..f6b45a0f --- /dev/null +++ b/Bloxstrap/Exceptions/InvalidHTTPResponseException.cs @@ -0,0 +1,13 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Bloxstrap.Exceptions +{ + internal class InvalidHTTPResponseException : Exception + { + public InvalidHTTPResponseException(string message) : base(message) { } + } +} diff --git a/Bloxstrap/Integrations/ActivityWatcher.cs b/Bloxstrap/Integrations/ActivityWatcher.cs index 94b68550..914493be 100644 --- a/Bloxstrap/Integrations/ActivityWatcher.cs +++ b/Bloxstrap/Integrations/ActivityWatcher.cs @@ -1,4 +1,6 @@ -namespace Bloxstrap.Integrations +using System.Windows; + +namespace Bloxstrap.Integrations { public class ActivityWatcher : IDisposable { @@ -372,21 +374,19 @@ public async Task GetServerLocation() string location = ""; var ipInfo = await Http.GetJson($"https://ipinfo.io/{Data.MachineAddress}/json"); - if (ipInfo is null) - return $"? ({Strings.ActivityTracker_LookupFailed})"; + if (String.IsNullOrEmpty(ipInfo.City)) + throw new InvalidHTTPResponseException("Reported city was blank"); - if (string.IsNullOrEmpty(ipInfo.Country)) - location = "?"; - else if (ipInfo.City == ipInfo.Region) + if (ipInfo.City == ipInfo.Region) location = $"{ipInfo.Region}, {ipInfo.Country}"; else location = $"{ipInfo.City}, {ipInfo.Region}, {ipInfo.Country}"; - if (!InGame) - return $"? ({Strings.ActivityTracker_LeftGame})"; - GeolocationCache[Data.MachineAddress] = location; + if (!InGame) + return ""; + return location; } catch (Exception ex) @@ -394,7 +394,9 @@ public async Task GetServerLocation() App.Logger.WriteLine(LOG_IDENT, $"Failed to get server location for {Data.MachineAddress}"); App.Logger.WriteException(LOG_IDENT, ex); - return $"? ({Strings.ActivityTracker_LookupFailed})"; + Frontend.ShowMessageBox($"{Strings.ActivityWatcher_LocationQueryFailed}\n\n{ex.Message}", MessageBoxImage.Warning); + + return "?"; } } diff --git a/Bloxstrap/Integrations/DiscordRichPresence.cs b/Bloxstrap/Integrations/DiscordRichPresence.cs index 306058ee..cc71527e 100644 --- a/Bloxstrap/Integrations/DiscordRichPresence.cs +++ b/Bloxstrap/Integrations/DiscordRichPresence.cs @@ -1,4 +1,6 @@ -using DiscordRPC; +using System.Windows; + +using DiscordRPC; namespace Bloxstrap.Integrations { @@ -204,17 +206,21 @@ public async Task SetCurrentGame() if (activity.UniverseDetails is null) { - await UniverseDetails.FetchSingle(activity.UniverseId); + try + { + await UniverseDetails.FetchSingle(activity.UniverseId); + } + catch (Exception ex) + { + App.Logger.WriteException(LOG_IDENT, ex); + Frontend.ShowMessageBox($"{Strings.ActivityWatcher_RichPresenceLoadFailed}\n\n{ex.Message}", MessageBoxImage.Warning); + return false; + } + activity.UniverseDetails = UniverseDetails.LoadFromCache(activity.UniverseId); } - var universeDetails = activity.UniverseDetails; - - if (universeDetails is null) - { - Frontend.ShowMessageBox(Strings.ActivityTracker_RichPresenceLoadFailed, System.Windows.MessageBoxImage.Warning); - return false; - } + var universeDetails = activity.UniverseDetails!; icon = universeDetails.Thumbnail.ImageUrl; diff --git a/Bloxstrap/JsonManager.cs b/Bloxstrap/JsonManager.cs index cf4d66ae..8e786bee 100644 --- a/Bloxstrap/JsonManager.cs +++ b/Bloxstrap/JsonManager.cs @@ -46,7 +46,7 @@ public virtual void Load(bool alertFailure = true) message = Strings.JsonManager_FastFlagsLoadFailed; if (!String.IsNullOrEmpty(message)) - Frontend.ShowMessageBox($"{message}\n\n{ex.GetType()}: {ex.Message}", System.Windows.MessageBoxImage.Warning); + Frontend.ShowMessageBox($"{message}\n\n{ex.Message}", System.Windows.MessageBoxImage.Warning); } Save(); diff --git a/Bloxstrap/Models/UniverseDetails.cs b/Bloxstrap/Models/UniverseDetails.cs index cdf4ec54..aa87501a 100644 --- a/Bloxstrap/Models/UniverseDetails.cs +++ b/Bloxstrap/Models/UniverseDetails.cs @@ -21,17 +21,19 @@ public class UniverseDetails return null; } - public static Task FetchSingle(long id) => FetchBulk(id.ToString()); + public static Task FetchSingle(long id) => FetchBulk(id.ToString()); - public static async Task FetchBulk(string ids) + public static async Task FetchBulk(string ids) { var gameDetailResponse = await Http.GetJson>($"https://games.roblox.com/v1/games?universeIds={ids}"); - if (gameDetailResponse is null || !gameDetailResponse.Data.Any()) - return false; + + if (!gameDetailResponse.Data.Any()) + throw new InvalidHTTPResponseException("Roblox API for Game Details returned invalid data"); var universeThumbnailResponse = await Http.GetJson>($"https://thumbnails.roblox.com/v1/games/icons?universeIds={ids}&returnPolicy=PlaceHolder&size=128x128&format=Png&isCircular=false"); - if (universeThumbnailResponse is null || !universeThumbnailResponse.Data.Any()) - return false; + + if (!universeThumbnailResponse.Data.Any()) + throw new InvalidHTTPResponseException("Roblox API for Game Thumbnails returned invalid data"); foreach (string strId in ids.Split(',')) { @@ -43,8 +45,6 @@ public static async Task FetchBulk(string ids) Thumbnail = universeThumbnailResponse.Data.Where(x => x.TargetId == id).First(), }); } - - return true; } } } diff --git a/Bloxstrap/Resources/Strings.Designer.cs b/Bloxstrap/Resources/Strings.Designer.cs index 765b5dcd..bf9ae723 100644 --- a/Bloxstrap/Resources/Strings.Designer.cs +++ b/Bloxstrap/Resources/Strings.Designer.cs @@ -106,29 +106,20 @@ public static string About_Translators_Title { } /// - /// Looks up a localized string similar to left game. + /// Looks up a localized string similar to Failed to query server location.. /// - public static string ActivityTracker_LeftGame { + public static string ActivityWatcher_LocationQueryFailed { get { - return ResourceManager.GetString("ActivityTracker.LeftGame", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to lookup failed. - /// - public static string ActivityTracker_LookupFailed { - get { - return ResourceManager.GetString("ActivityTracker.LookupFailed", resourceCulture); + return ResourceManager.GetString("ActivityWatcher.LocationQueryFailed", resourceCulture); } } /// /// Looks up a localized string similar to Your current game will not show on your Discord presence because an error occurred when loading the game information.. /// - public static string ActivityTracker_RichPresenceLoadFailed { + public static string ActivityWatcher_RichPresenceLoadFailed { get { - return ResourceManager.GetString("ActivityTracker.RichPresenceLoadFailed", resourceCulture); + return ResourceManager.GetString("ActivityWatcher.RichPresenceLoadFailed", resourceCulture); } } diff --git a/Bloxstrap/Resources/Strings.resx b/Bloxstrap/Resources/Strings.resx index 3510b27a..85bd3321 100644 --- a/Bloxstrap/Resources/Strings.resx +++ b/Bloxstrap/Resources/Strings.resx @@ -117,12 +117,6 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - left game - - - lookup failed - Bloxstrap was unable to automatically update to version {0}. Please update it manually by downloading and running it from the website. @@ -1171,10 +1165,13 @@ Are you sure you want to continue? Rejoin - + Your current game will not show on your Discord presence because an error occurred when loading the game information. Game history is only recorded for your current Roblox session. Games will appear here as you leave them or teleport within them. + + Failed to query server location. + \ No newline at end of file diff --git a/Bloxstrap/UI/Elements/ContextMenu/ServerHistory.xaml b/Bloxstrap/UI/Elements/ContextMenu/ServerHistory.xaml index 2b49385b..a373896f 100644 --- a/Bloxstrap/UI/Elements/ContextMenu/ServerHistory.xaml +++ b/Bloxstrap/UI/Elements/ContextMenu/ServerHistory.xaml @@ -8,6 +8,7 @@ xmlns:ui="http://schemas.lepo.co/wpfui/2022/xaml" xmlns:models="clr-namespace:Bloxstrap.UI.ViewModels.ContextMenu" xmlns:resources="clr-namespace:Bloxstrap.Resources" + xmlns:enums="clr-namespace:Bloxstrap.Enums" d:DataContext="{d:DesignInstance Type=models:ServerHistoryViewModel}" mc:Ignorable="d" Title="{x:Static resources:Strings.ContextMenu_GameHistory_Title}" @@ -30,11 +31,24 @@ + + + + + + diff --git a/Bloxstrap/UI/NotifyIconWrapper.cs b/Bloxstrap/UI/NotifyIconWrapper.cs index cf1dd785..59ac16e0 100644 --- a/Bloxstrap/UI/NotifyIconWrapper.cs +++ b/Bloxstrap/UI/NotifyIconWrapper.cs @@ -60,6 +60,10 @@ public async void OnGameJoin(object? sender, EventArgs e) return; string serverLocation = await _activityWatcher.GetServerLocation(); + + if (string.IsNullOrEmpty(serverLocation)) + return; + string title = _activityWatcher.Data.ServerType switch { ServerType.Public => Strings.ContextMenu_ServerInformation_Notification_Title_Public, diff --git a/Bloxstrap/UI/ViewModels/ContextMenu/ServerHistoryViewModel.cs b/Bloxstrap/UI/ViewModels/ContextMenu/ServerHistoryViewModel.cs index ead0033e..bea9d615 100644 --- a/Bloxstrap/UI/ViewModels/ContextMenu/ServerHistoryViewModel.cs +++ b/Bloxstrap/UI/ViewModels/ContextMenu/ServerHistoryViewModel.cs @@ -10,6 +10,10 @@ internal class ServerHistoryViewModel : NotifyPropertyChangedViewModel public List? GameHistory { get; private set; } + public GenericTriState LoadState { get; private set; } = GenericTriState.Unknown; + + public string Error { get; private set; } = String.Empty; + public ICommand CloseWindowCommand => new RelayCommand(RequestClose); public EventHandler? RequestCloseEvent; @@ -25,6 +29,9 @@ public ServerHistoryViewModel(ActivityWatcher activityWatcher) private async void LoadData() { + LoadState = GenericTriState.Unknown; + OnPropertyChanged(nameof(LoadState)); + var entries = _activityWatcher.History.Where(x => x.UniverseDetails is null); if (entries.Any()) @@ -32,8 +39,22 @@ private async void LoadData() // TODO: this will duplicate universe ids string universeIds = String.Join(',', entries.Select(x => x.UniverseId)); - if (!await UniverseDetails.FetchBulk(universeIds)) + try + { + await UniverseDetails.FetchBulk(universeIds); + } + catch (Exception ex) + { + App.Logger.WriteException("ServerHistoryViewModel::LoadData", ex); + + Error = ex.Message; + OnPropertyChanged(nameof(Error)); + + LoadState = GenericTriState.Failed; + OnPropertyChanged(nameof(LoadState)); + return; + } foreach (var entry in entries) entry.UniverseDetails = UniverseDetails.LoadFromCache(entry.UniverseId); @@ -64,6 +85,9 @@ private async void LoadData() } OnPropertyChanged(nameof(GameHistory)); + + LoadState = GenericTriState.Successful; + OnPropertyChanged(nameof(LoadState)); } private void RequestClose() => RequestCloseEvent?.Invoke(this, EventArgs.Empty); diff --git a/Bloxstrap/Utilities.cs b/Bloxstrap/Utilities.cs index 9382c9a0..4613d016 100644 --- a/Bloxstrap/Utilities.cs +++ b/Bloxstrap/Utilities.cs @@ -5,14 +5,6 @@ namespace Bloxstrap { static class Utilities { - /// - /// Is process running as administrator - /// https://stackoverflow.com/a/11660205 - /// - public static bool IsAdministrator => - new WindowsPrincipal(WindowsIdentity.GetCurrent()) - .IsInRole(WindowsBuiltInRole.Administrator); - public static void ShellExecute(string website) { try diff --git a/Bloxstrap/Utility/Http.cs b/Bloxstrap/Utility/Http.cs index f0532082..f95b3073 100644 --- a/Bloxstrap/Utility/Http.cs +++ b/Bloxstrap/Utility/Http.cs @@ -2,22 +2,22 @@ { internal static class Http { - public static async Task GetJson(string url) + /// + /// Gets and deserializes a JSON API response to the specified object + /// + /// + /// + /// + /// + public static async Task GetJson(string url) { - string LOG_IDENT = $"Http::GetJson<{typeof(T).Name}>"; + var request = await App.HttpClient.GetAsync(url); - string json = await App.HttpClient.GetStringAsync(url); + request.EnsureSuccessStatusCode(); - try - { - return JsonSerializer.Deserialize(json); - } - catch (Exception ex) - { - App.Logger.WriteLine(LOG_IDENT, $"Failed to deserialize JSON for {url}!"); - App.Logger.WriteException(LOG_IDENT, ex); - return default; - } + string json = await request.Content.ReadAsStringAsync(); + + return JsonSerializer.Deserialize(json)!; } } }