Merge pull request #1286 from slskd/search

jpdillingham · web-flow · commit a2e1e65b7f85 · 2025-01-29T23:28:57.000-06:00
Fix 'stuck' searches, including those that contain a single letter
diff --git a/src/slskd/Search/API/Controllers/SearchesController.cs b/src/slskd/Search/API/Controllers/SearchesController.cs
@@ -24,6 +24,7 @@ namespace slskd.Search.API
     using Asp.Versioning;
     using Microsoft.AspNetCore.Authorization;
     using Microsoft.AspNetCore.Mvc;
+    using Serilog;
     using SearchQuery = Soulseek.SearchQuery;
     using SearchScope = Soulseek.SearchScope;
 
@@ -50,6 +51,7 @@ public SearchesController(ISearchService searchService, IOptionsSnapshot<Options
 
         private ISearchService Searches { get; }
         private IOptionsSnapshot<Options> OptionsSnapshot { get; }
+        private ILogger Log { get; set; } = Serilog.Log.ForContext<SearchesController>();
 
         /// <summary>
         ///     Performs a search for the specified <paramref name="request"/>.
@@ -83,12 +85,19 @@ public async Task<IActionResult> Post([FromBody] SearchRequest request)
             }
             catch (Exception ex) when (ex is ArgumentException || ex is Soulseek.DuplicateTokenException)
             {
+                Log.Error(ex, "Failed to execute search {Search}: {Message}", request, ex.Message);
                 return BadRequest(ex.Message);
             }
             catch (InvalidOperationException ex)
             {
+                Log.Error(ex, "Failed to execute search {Search}: {Message}", request, ex.Message);
                 return Conflict(ex.Message);
             }
+            catch (Exception ex)
+            {
+                Log.Error(ex, "Failed to execute search {Search}: {Message}", request, ex.Message);
+                return StatusCode(500, ex.Message);
+            }
 
             return Ok(search);
         }
diff --git a/src/slskd/Search/SearchService.cs b/src/slskd/Search/SearchService.cs
@@ -228,6 +228,8 @@ public async Task<Search> StartAsync(Guid id, SearchQuery query, SearchScope sco
 
             var rateLimiter = new RateLimiter(250);
 
+            // initialize the search record, save it to the database, and broadcast the creation
+            // we do this so the UI has some feedback to show to the user that we've gotten their request
             var search = new Search()
             {
                 SearchText = query.SearchText,
@@ -241,6 +243,10 @@ public async Task<Search> StartAsync(Guid id, SearchQuery query, SearchScope sco
             context.Add(search);
             context.SaveChanges();
 
+            await SearchHub.BroadcastCreateAsync(search);
+
+            // initialize the list of responses that we'll use to accumulate them
+            // populated by the responseHandler we pass to SearchAsync
             List<SearchResponse> responses = new();
 
             options ??= new SearchOptions();
@@ -266,47 +272,75 @@ public async Task<Search> StartAsync(Guid id, SearchQuery query, SearchScope sco
                     Update(search);
                 }));
 
-            var soulseekSearchTask = Client.SearchAsync(
-                query,
-                responseHandler: (response) => responses.Add(response),
-                scope,
-                token,
-                options,
-                cancellationToken: cancellationTokenSource.Token);
-
-            _ = Task.Run(async () =>
+            try
             {
-                try
-                {
-                    var soulseekSearch = await soulseekSearchTask;
-                    search = search.WithSoulseekSearch(soulseekSearch);
-                }
-                finally
+                // initiate the search. this can throw at invocation if there's a problem with
+                // the client state (e.g. disconnected) or a problem with the search (e.g. no terms)
+                var soulseekSearchTask = Client.SearchAsync(
+                    query,
+                    responseHandler: (response) => responses.Add(response),
+                    scope,
+                    token,
+                    options,
+                    cancellationToken: cancellationTokenSource.Token);
+
+                // seach looks ok so far; let the rest of the logic run asynchronously
+                // on a background thread. this logic needs to clean up after itself and
+                // update the search record to accurately reflect the final state
+                _ = Task.Run(async () =>
                 {
-                    rateLimiter.Dispose();
-                    CancellationTokens.TryRemove(id, out _);
-
                     try
                     {
-                        search.EndedAt = DateTime.UtcNow;
-                        search.Responses = responses.Select(r => Response.FromSoulseekSearchResponse(r));
-
-                        Update(search);
-
-                        // zero responses before broadcasting, as we don't want to blast this
-                        // data out over the SignalR socket
-                        await SearchHub.BroadcastUpdateAsync(search with { Responses = [] });
+                        var soulseekSearch = await soulseekSearchTask;
+                        search = search.WithSoulseekSearch(soulseekSearch);
                     }
                     catch (Exception ex)
                     {
-                        Log.Error(ex, "Failed to persist search for {SearchQuery} ({Id})", query, id);
+                        Log.Error(ex, "Failed to execute search {Search}: {Message}", new { query, scope, options }, ex.Message);
+                        search.State = SearchStates.Completed | SearchStates.Errored;
                     }
-                }
-            });
+                    finally
+                    {
+                        rateLimiter.Dispose();
+                        CancellationTokens.TryRemove(id, out _);
+
+                        try
+                        {
+                            search.EndedAt = DateTime.UtcNow;
+                            search.Responses = responses.Select(r => Response.FromSoulseekSearchResponse(r));
+
+                            Update(search);
+
+                            // zero responses before broadcasting, as we don't want to blast this
+                            // data out over the SignalR socket
+                            await SearchHub.BroadcastUpdateAsync(search with { Responses = [] });
+                        }
+                        catch (Exception ex)
+                        {
+                            // record may be left 'hanging' and will need to be cleaned up at the next boot
+                            Log.Error(ex, "Failed to persist search for {SearchQuery} ({Id})", query, id);
+                        }
+                    }
+                });
 
-            await SearchHub.BroadcastCreateAsync(search);
+                await SearchHub.BroadcastUpdateAsync(search);
+
+                return search;
+            }
+            catch (Exception ex)
+            {
+                // we'll end up here if the initial call throws for an ArgumentException, InvalidOperationException if
+                // the app isn't connected, and a few other straightforward issues that arise before even requesting the search
+                Log.Error(ex, "Failed to execute search {Search}: {Message}", new { query, scope, options }, ex.Message);
+
+                search.State = SearchStates.Completed | SearchStates.Errored;
+                search.EndedAt = search.StartedAt;
+                Update(search);
+
+                await SearchHub.BroadcastUpdateAsync(search with { Responses = [] });
 
-            return search;
+                throw;
+            }
         }
 
         /// <summary>
diff --git a/src/web/src/components/Search/Searches.jsx b/src/web/src/components/Search/Searches.jsx
@@ -72,7 +72,9 @@ const Searches = ({ server } = {}) => {
       });
     });
 
-    searchHub.on('create', () => {});
+    searchHub.on('create', (search) => {
+      onUpdate((old) => ({ ...old, [search.id]: search }));
+    });
 
     searchHub.onreconnecting((connectionError) =>
       onConnectionError(connectionError?.message ?? 'Disconnected'),

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ namespace slskd.Search.API`
`24`	`24`	`using Asp.Versioning;`
`25`	`25`	`using Microsoft.AspNetCore.Authorization;`
`26`	`26`	`using Microsoft.AspNetCore.Mvc;`
	`27`	`+ using Serilog;`
`27`	`28`	`using SearchQuery = Soulseek.SearchQuery;`
`28`	`29`	`using SearchScope = Soulseek.SearchScope;`
`29`	`30`
`@@ -50,6 +51,7 @@ public SearchesController(ISearchService searchService, IOptionsSnapshot<Options`
`50`	`51`
`51`	`52`	`private ISearchService Searches { get; }`
`52`	`53`	`private IOptionsSnapshot<Options> OptionsSnapshot { get; }`
	`54`	`+ private ILogger Log { get; set; } = Serilog.Log.ForContext<SearchesController>();`
`53`	`55`
`54`	`56`	`/// <summary>`
`55`	`57`	`/// Performs a search for the specified <paramref name="request"/>.`
`@@ -83,12 +85,19 @@ public async Task<IActionResult> Post([FromBody] SearchRequest request)`
`83`	`85`	`}`
`84`	`86`	`catch (Exception ex) when (ex is ArgumentException \|\| ex is Soulseek.DuplicateTokenException)`
`85`	`87`	`{`
	`88`	`+ Log.Error(ex, "Failed to execute search {Search}: {Message}", request, ex.Message);`
`86`	`89`	`return BadRequest(ex.Message);`
`87`	`90`	`}`
`88`	`91`	`catch (InvalidOperationException ex)`
`89`	`92`	`{`
	`93`	`+ Log.Error(ex, "Failed to execute search {Search}: {Message}", request, ex.Message);`
`90`	`94`	`return Conflict(ex.Message);`
`91`	`95`	`}`
	`96`	`+ catch (Exception ex)`
	`97`	`+ {`
	`98`	`+ Log.Error(ex, "Failed to execute search {Search}: {Message}", request, ex.Message);`
	`99`	`+ return StatusCode(500, ex.Message);`
	`100`	`+ }`
`92`	`101`
`93`	`102`	`return Ok(search);`
`94`	`103`	`}`