fix(poracle): a refused human or profile write is the caller's error, not a 500

Applications/Pgan.PoracleWebNet.Api/Filters/PoracleRequestRefusedExceptionFilter.cs

@@ -0,0 +1,37 @@
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Filters;
+using Pgan.PoracleWebNet.Core.Models;
+
+namespace Pgan.PoracleWebNet.Api.Filters;
+
+/// <summary>
+/// Reports a refusal from PoracleNG's human, profile, area and location routes as the caller's problem.
+/// </summary>
+/// <remarks>
+/// Registered globally beside <see cref="AlarmValidationExceptionFilter"/>, which does the same job for the
+/// tracking routes. Same body shape as every other refusal in this API -- <c>{ "error": "..." }</c> -- so
+/// the SPA's interceptor needs nothing new to show it.
+/// </remarks>
+public sealed class PoracleRequestRefusedExceptionFilter : IActionFilter
+{
+    public void OnActionExecuting(ActionExecutingContext context)
+    {
+    }
+
+    public void OnActionExecuted(ActionExecutedContext context)
+    {
+        if (context.Exception is not PoracleRequestRefusedException ex)
+        {
+            return;
+        }
+
+        context.Result = new ObjectResult(new
+        {
+            error = ex.Message,
+        })
+        {
+            StatusCode = ex.StatusCode,
+        };
+        context.ExceptionHandled = true;
+    }
+}

Applications/Pgan.PoracleWebNet.Api/Program.cs

@@ -188,6 +188,7 @@
     options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.SummaryBackendUnavailableExceptionFilter>();
     options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.TrackingConflictExceptionFilter>();
     options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.AlarmValidationExceptionFilter>();
+    options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.PoracleRequestRefusedExceptionFilter>();
     options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.TrackingRuleNotFoundExceptionFilter>();
     options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.AccountGoneExceptionFilter>();
     options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.PoracleUnsupportedExceptionFilter>();

CHANGELOG.md

@@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A refused profile, area, place or account change now says what was wrong with it.** Saving a profile with no number, a location that is not a location, an area list Poracle could not read, or banning a user through an older path all came back as *An unexpected error occurred* -- the wording this site uses when it genuinely does not know -- and were recorded as server faults. Poracle had explained every one of them; the explanation was being thrown away one layer below the screen. It now reaches the dialog that asked, in Poracle's own words, whether the server answered in its long-standing format or the newer one 5.2.1 introduced. The alarm pages were fixed this way some time ago; this is the same fix on the half of the site it never reached.
+- **Switching to a profile that is no longer there stops signing you out.** Any *not found* answer from Poracle was read as *your account has been deleted*, which is what one of them means. A missing profile is not, and neither is a route an older Poracle does not have -- but both ended the session and sent you back to the login page, and one of them did it while a profile duplicate was tidying up after itself, hiding whatever had actually gone wrong. A deleted account still signs you out, exactly as before.
 - **An underscore in a name survives the rule sentence on an alarm card.** The sentence was having every `*`, `_` and backtick stripped out of it, not only the ones acting as Discord emphasis. Nothing Poracle renders today contains one, so there is no visible change: this is the case that had not arrived yet. Poracle interpolates template names, areas and saved-place labels into that sentence without escaping them, so an area called `north_side` would have reached the card as `northside`. Emphasis is now removed only where it is paired, and where an underscore sits between two word characters it is left alone -- which is what Discord does with it too ([#819](https://github.com/PGAN-Dev/PoracleWeb.NET/pull/819)).
 - **Pokéstop Events works at all.** The page shipped unable to answer a single request: the service behind it was never handed to the application at startup, so opening the page, adding an event or deleting one all failed the same way, with the generic server error. Nothing caught it -- the tests for that code substitute the missing piece, so they passed, and the site compiled and deployed green. A new test now builds every page's dependencies the way the running application does, which is the only place this kind of omission shows up.
 - **The PVP rank range is readable again on a dark-themed alarm card.** The band under a PVP alarm showed its league and nothing else, so the ranks you had set looked like they had been dropped. They were being drawn, in white, on a band that stays light in both themes. The league name gained some contrast on the way past ([#800](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/800)).

Core/Pgan.PoracleWebNet.Core.Models/PoracleRequestRefusedException.cs

@@ -0,0 +1,50 @@
+namespace Pgan.PoracleWebNet.Core.Models;
+
+/// <summary>
+/// PoracleNG refused a human, profile, area or location request and said why.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <c>PoracleHumanProxy</c> handled 404 and one 409 and let everything else reach
+/// <c>EnsureSuccessStatusCode()</c>, so a 400 such as "state is required (true/false)" or "profile_no must
+/// be specified" arrived at the browser as 500 "An unexpected error occurred" and was logged as a server
+/// fault. This is the same defect #539 closed on the tracking proxy, on the half of the surface that fix
+/// did not reach.
+/// </para>
+/// <para>
+/// Deliberately not <see cref="AlarmValidationException"/>: that type is documented as an alarm rejection
+/// and shares an alarm-worded fallback with it, so reusing it would answer "Poracle rejected the alarm."
+/// to someone renaming a profile or saving a place. One narrowly-named exception per failure shape is the
+/// pattern the rest of the filters already follow.
+/// </para>
+/// <para>
+/// <see cref="StatusCode"/> carries the answer through because not every refusal is a 400: PoracleNG
+/// answers 404 "Profile not found" for a profile number that does not exist, which is the caller naming
+/// something absent, not the account being gone.
+/// </para>
+/// </remarks>
+public sealed class PoracleRequestRefusedException : Exception
+{
+    public PoracleRequestRefusedException(string message)
+        : base(message)
+    {
+    }
+
+    public PoracleRequestRefusedException(string message, int statusCode)
+        : base(message) => this.StatusCode = statusCode;
+
+    public PoracleRequestRefusedException()
+    {
+    }
+
+    public PoracleRequestRefusedException(string message, Exception innerException)
+        : base(message, innerException)
+    {
+    }
+
+    /// <summary>The status the API should answer. 400 unless PoracleNG named something absent.</summary>
+    public int StatusCode
+    {
+        get; init;
+    } = 400;
+}

Core/Pgan.PoracleWebNet.Core.Services/PoracleHumanProxy.cs

@@ -9,6 +9,9 @@ namespace Pgan.PoracleWebNet.Core.Services;
 
 public class PoracleHumanProxy(HttpClient httpClient, IConfiguration configuration) : IPoracleHumanProxy
 {
+    /// <summary>What to say when PoracleNG refused and explained nothing usable.</summary>
+    private const string Unexplained = "Poracle rejected the request.";
+
     private readonly HttpClient _httpClient = httpClient;
     private readonly string _apiAddress = configuration["Poracle:ApiAddress"] ?? string.Empty;
     private readonly string _apiSecret = configuration["Poracle:ApiSecret"] ?? string.Empty;
@@ -17,23 +20,75 @@ public class PoracleHumanProxy(HttpClient httpClient, IConfiguration configurati
     /// URL-encodes a userId for safe path construction. Webhook IDs are full URLs
     /// containing slashes that would break routing without encoding.
     /// </summary>
+    private static string Encode(string userId) => Uri.EscapeDataString(userId);
+
     /// <summary>
-    /// Turns PoracleNG's "user not found" into something the API can answer 401 to.
+    /// Turns a refusal from PoracleNG into the answer it deserves, and lets everything else throw.
     /// </summary>
     /// <remarks>
-    /// A JWT outlives the account it names. Without this, every lookup for a deleted user threw an
-    /// HttpRequestException that the global handler flattened into a 500, so the SPA -- which signs out
-    /// only on 401 -- left the user in an app where every page failed. See #584.
+    /// <para>
+    /// Only 404 was read here, and one 409 on the delete-place path. Everything else fell through to
+    /// <c>EnsureSuccessStatusCode()</c>, whose <see cref="HttpRequestException"/> the global handler
+    /// flattens into 500 "An unexpected error occurred" -- so "state is required (true/false)",
+    /// "profile_no must be specified" and "invalid latitude" all reached the user as a server fault and
+    /// were logged as one. #539 fixed exactly this on the tracking proxy and never reached here.
+    /// </para>
+    /// <para>
+    /// 422 is the same refusal wearing a different number. Verified live: the v1 human and profile routes
+    /// answer 400 on 5.1.0 and 5.2.1 alike, but 5.2.1's <c>/api/v2/humans</c> surface answers RFC 9457
+    /// problem+json at 422 for the same mistakes. Matching only 400 would re-open this the day a call
+    /// site moves to v2.
+    /// </para>
+    /// <para>
+    /// A 404 is account-gone only when PoracleNG says so. It also answers 404 "Profile not found" when a
+    /// profile number does not exist, and gin answers a plaintext "404 page not found" for a route this
+    /// build does not have -- treating either as a dead account signed the user out of a working session,
+    /// and in <c>ProfileOverviewService</c>'s restore-the-profile <c>finally</c> it did so while
+    /// swallowing the real failure. Both verified against 5.1.0 and 5.2.1.
+    /// </para>
     /// </remarks>
-    private static void EnsureAccountStillExists(HttpResponseMessage response)
+    private static async Task EnsureAcceptedAsync(HttpResponseMessage response)
     {
-        if (response.StatusCode == HttpStatusCode.NotFound)
+        if (response.IsSuccessStatusCode)
+        {
+            return;
+        }
+
+        var payload = await response.Content.ReadAsStringAsync();
+
+        switch (response.StatusCode)
         {
-            throw new AccountGoneException();
+            case HttpStatusCode.NotFound when NamesAMissingAccount(payload):
+                // A JWT outlives the account it names. Without this, every lookup for a deleted user threw
+                // an HttpRequestException that the global handler flattened into a 500, so the SPA -- which
+                // signs out only on 401 -- left the user in an app where every page failed. See #584.
+                throw new AccountGoneException();
+
+            case HttpStatusCode.NotFound:
+                throw new PoracleRequestRefusedException(
+                    PoracleProblemDetails.Describe(payload, Unexplained), (int)HttpStatusCode.NotFound);
+
+            case HttpStatusCode.BadRequest:
+            case HttpStatusCode.UnprocessableEntity:
+                throw new PoracleRequestRefusedException(PoracleProblemDetails.Describe(payload, Unexplained));
+
+            default:
+                response.EnsureSuccessStatusCode();
+                return;
         }
     }
 
-    private static string Encode(string userId) => Uri.EscapeDataString(userId);
+    /// <summary>
+    /// True when a 404 body is PoracleNG saying the account itself is gone rather than something in it.
+    /// </summary>
+    /// <remarks>
+    /// v1 answers <c>{"message":"User not found"}</c>; the v2 surface and the tracking routes say
+    /// "human not found". Anything else at 404 names a profile, a place or a route.
+    /// </remarks>
+    private static bool NamesAMissingAccount(string? payload) =>
+        payload is not null
+        && (payload.Contains("user not found", StringComparison.OrdinalIgnoreCase)
+            || payload.Contains("human not found", StringComparison.OrdinalIgnoreCase));
 
     public async Task<JsonElement?> GetHumanAsync(string userId)
     {
@@ -58,22 +113,19 @@ private static void EnsureAccountStillExists(HttpResponseMessage response)
     public async Task CreateHumanAsync(JsonElement body)
     {
         var response = await this.SendAsync(HttpMethod.Post, "/api/humans", body.GetRawText());
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task StartAsync(string userId)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/start");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task StopAsync(string userId)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/stop");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task AdminDisabledAsync(string userId, bool disabled)
@@ -86,23 +138,20 @@ public async Task AdminDisabledAsync(string userId, bool disabled)
             state = disabled
         });
         var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/adminDisabled", body);
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task SetLocationAsync(string userId, double lat, double lon)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/setLocation/{lat}/{lon}");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task SetAreasAsync(string userId, string[] areas)
     {
         var body = JsonSerializer.Serialize(areas);
         var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/setAreas", body);
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task<JsonElement?> GetAreasAsync(string userId) =>
@@ -113,15 +162,13 @@ public async Task SetAreasAsync(string userId, string[] areas)
     public async Task SwitchProfileAsync(string userId, int profileNo)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/switchProfile/{profileNo}");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task<JsonElement> GetProfilesAsync(string userId)
     {
         var response = await this.SendAsync(HttpMethod.Get, $"/api/profiles/{Encode(userId)}");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
 
         var json = await response.Content.ReadAsStringAsync();
         using var doc = JsonDocument.Parse(json);
@@ -131,29 +178,25 @@ public async Task<JsonElement> GetProfilesAsync(string userId)
     public async Task AddProfileAsync(string userId, JsonElement body)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/profiles/{Encode(userId)}/add", body.GetRawText());
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task UpdateProfileAsync(string userId, JsonElement body)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/profiles/{Encode(userId)}/update", body.GetRawText());
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task DeleteProfileAsync(string userId, int profileNo)
     {
         var response = await this.SendAsync(HttpMethod.Delete, $"/api/profiles/{Encode(userId)}/byProfileNo/{profileNo}");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task CopyProfileAsync(string userId, int fromProfileNo, int toProfileNo)
     {
         var response = await this.SendAsync(HttpMethod.Post, $"/api/profiles/{Encode(userId)}/copy/{fromProfileNo}/{toProfileNo}");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     public async Task<JsonElement?> CheckLocationAsync(string userId, double lat, double lon)
@@ -173,8 +216,7 @@ public async Task CopyProfileAsync(string userId, int fromProfileNo, int toProfi
     public async Task<SavedPlaces> GetPlacesAsync(string userId)
     {
         var response = await this.SendAsync(HttpMethod.Get, $"/api/humans/{Encode(userId)}/locations");
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
 
         var json = await response.Content.ReadAsStringAsync();
         using var doc = JsonDocument.Parse(json);
@@ -225,8 +267,7 @@ public async Task<SavedPlaces> GetPlacesAsync(string userId)
 
         var response = await this.SendAsync(
             HttpMethod.Post, $"/api/humans/{Encode(userId)}/locations/add", body);
-        EnsureAccountStillExists(response);
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
 
         // A rejected label is reported inside a 200: PoracleNG answers per row so a batch can partly
         // succeed. Treating the 200 as success stored nothing and told the user it worked.
@@ -255,8 +296,9 @@ public async Task DeletePlaceAsync(string userId, string label)
     {
         var response = await this.SendAsync(
             HttpMethod.Post, $"/api/humans/{Encode(userId)}/locations/{Encode(label)}/delete");
-        EnsureAccountStillExists(response);
 
+        // Read before the general refusal path: a 409 here names the alarms still pointing at the place,
+        // which is the difference between "could not delete" and knowing what to repoint first.
         if (response.StatusCode == HttpStatusCode.Conflict)
         {
             var conflict = await response.Content.ReadAsStringAsync();
@@ -269,7 +311,7 @@ public async Task DeletePlaceAsync(string userId, string label)
             throw new PlaceInUseException(rules);
         }
 
-        response.EnsureSuccessStatusCode();
+        await EnsureAcceptedAsync(response);
     }
 
     private async Task<HttpResponseMessage> SendAsync(HttpMethod method, string path, string? body = null)