fix: client SDK release review findings

.github/workflows/sdk-release.yml

@@ -102,8 +102,8 @@ jobs:
       run:
         working-directory: sdk/node-client
     steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-node@v4
+      - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
         with:
           node-version: "22"
           registry-url: "https://registry.npmjs.org"

SW.Bitween.Client/BitweenClient.cs

@@ -84,7 +84,15 @@ internal async Task PostAsync(string path, object body, CancellationToken cancel
     internal async Task<Stream> PostForStreamAsync(string path, object body, CancellationToken cancellation)
     {
         var response = await SendAsync(() => Post(path, body), HttpCompletionOption.ResponseHeadersRead, cancellation);
-        return await response.Content.ReadAsStreamAsync(cancellation);
+        try
+        {
+            return await response.Content.ReadAsStreamAsync(cancellation);
+        }
+        catch
+        {
+            response.Dispose();
+            throw;
+        }
     }
 
     static HttpRequestMessage Post(string path, object body) =>

SW.Bitween.Client/ExchangeQuery.cs

@@ -58,8 +58,8 @@ public class ExchangeQuery
     public string? ErrorContains { get; set; }
 
     /// <summary>
-    /// Promoted properties by key: the value contains the text, ignoring case, or with a null text,
-    /// the exchange promoted the key at all. Every one has to hold.
+    /// Promoted properties by key: the value contains the text, ignoring case, or with a null or blank
+    /// text, the exchange promoted the key at all. Every one has to hold.
     /// </summary>
     public IDictionary<string, string?>? PromotedProperties { get; set; }
 
@@ -132,6 +132,9 @@ public sealed class ExchangeSelection
     /// <summary>Every exchange <paramref name="query"/> matches, but those in <paramref name="except"/>.</summary>
     public static ExchangeSelection Matching(ExchangeQuery query, IEnumerable<string>? except = null)
     {
+        // No ids adds no filter, so it would select every exchange rather than none.
+        if (query.Ids is { Count: 0 })
+            throw new ArgumentException("Ids is empty: it would match every exchange.", nameof(query));
         var filters = query.ToFilters();
         // An empty filter reads as no filter, and the selection would fall back to its ids, which are
         // none. Every attempt, not only the newest, matches everything without narrowing it.
@@ -174,6 +177,9 @@ public sealed class Page<T>
 {
     public List<T> Result { get; set; } = [];
 
-    /// <summary>How many match. An exchange search stops counting at 10,001, meaning more than 10,000.</summary>
+    /// <summary>
+    /// How many match, or 0 when searched without counting. An exchange search stops counting at
+    /// 10,001, meaning more than 10,000.
+    /// </summary>
     public int TotalCount { get; set; }
 }

SW.Bitween.Client/Exchanges.cs

@@ -1,4 +1,3 @@
-using System.Net;
 using SW.Bitween.Model;
 
 namespace SW.Bitween.Client;
@@ -54,8 +53,8 @@ public Task<XchangeRetryTree> GetRetryTreeAsync(string id, CancellationToken can
         bitween.GetAsync<XchangeRetryTree>($"api/xchanges/retrytree?id={Uri.EscapeDataString(id)}", cancellation);
 
     /// <summary>
-    /// Runs an exchange again from its input file, and returns the new attempt's id: null when the
-    /// account may retry but not view exchanges, so can't look it up. With
+    /// Runs an exchange again from its input file, and returns the new attempt's id: null when it can't
+    /// look that up, because the account may retry but not view exchanges or the lookup failed. With
     /// <paramref name="reset"/>, the subscription's current settings are used rather than those the
     /// exchange ran with. An exchange is retried at most once: one that has been is refused with
     /// <c>ALREADY_RETRIED</c>, so retry its newest attempt instead. One with an automatic retry booked
@@ -68,13 +67,13 @@ public Task<XchangeRetryTree> GetRetryTreeAsync(string id, CancellationToken can
         await bitween.PostAsync($"api/xchanges/{Uri.EscapeDataString(id)}/retry",
             new XchangeRetry { Reason = reason, Reset = reset }, cancellation);
         // Answered with nothing: the new attempt is the newest that says it retries this. Looking it up
-        // needs exchanges.view, and the retry is made by then, so a refusal mustn't read as the retry's.
+        // needs exchanges.view, and the retry is made by then, so a failed lookup mustn't read as the retry's.
         try
         {
             var retry = await SearchAsync(new ExchangeQuery { RetryFor = id }, 0, 1, false, cancellation);
             return retry.Result.FirstOrDefault()?.Id;
         }
-        catch (BitweenClientException e) when (e.Status == HttpStatusCode.Forbidden)
+        catch (Exception e) when (e is BitweenClientException or HttpRequestException)
         {
             return null;
         }

SW.Bitween.HttpTests/BitweenClientTests.cs

@@ -4,7 +4,9 @@
 using System.IO.Compression;
 using System.Linq;
 using System.Net;
+using System.Net.Http;
 using System.Net.Http.Json;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore;
 using SW.Bitween.Client;
@@ -119,6 +121,9 @@ await fixture.InDbAsync(db => db.Set<Domain.XchangeResult>().Where(r => ids.Cont
         Assert.Equal(2, preview.WillRetry);
         Assert.Equal(3, (await bitween.Exchanges.SearchAsync(partners)).TotalCount);
 
+        // No ids would read as no filter, and select every exchange.
+        Assert.Throws<ArgumentException>(() => ExchangeSelection.Matching(new ExchangeQuery { Ids = [] }));
+
         var plan = await bitween.Exchanges.BulkRetryAsync(ExchangeSelection.Of(ids.Take(2)));
         Assert.Equal(2, plan.WillRetry);
         Assert.Equal(5, (await bitween.Exchanges.SearchAsync(partners)).TotalCount);
@@ -135,6 +140,26 @@ await fixture.InDbAsync(db => db.Set<Domain.XchangeResult>().Where(r => ids.Cont
         Assert.All(ids, id => Assert.Contains(archive.Entries, e => e.FullName.Contains(id)));
     }
 
+    [Fact]
+    public async Task Runs_a_scheduled_retry_now()
+    {
+        var (partnerId, _) = await ExchangesAsync(1);
+        using var bitween = await ClientAsync("exchanges.view", "exchanges.operate");
+        var id = (await bitween.Exchanges.SearchAsync(new ExchangeQuery { PartnerIds = [partnerId] })).Result.Single().Id;
+        // Booked as a retry policy books one, an hour out so the retry job leaves it.
+        await fixture.InDbAsync(db =>
+        {
+            db.Add(new Domain.DelayedRetry { Id = id, On = DateTime.UtcNow.AddHours(1) });
+            return db.SaveChangesAsync();
+        });
+        var booked = new ScheduledRetryQuery { From = DateTimeOffset.UtcNow };
+        Assert.Contains((await bitween.ScheduledRetries.SearchAsync(booked)).Result, r => r.Id == id);
+
+        await bitween.ScheduledRetries.RunNowAsync(id);
+        Assert.Single((await bitween.Exchanges.SearchAsync(new ExchangeQuery { RetryFor = id })).Result);
+        Assert.DoesNotContain((await bitween.ScheduledRetries.SearchAsync(booked)).Result, r => r.Id == id);
+    }
+
     [Fact]
     public async Task Says_what_Bitween_refused_and_why()
     {
@@ -155,7 +180,9 @@ public async Task Says_what_Bitween_refused_and_why()
 
         // Calls made at once with a bad token share its one refusal. Sent once a call, the fifth would
         // lock the address out, and a good token from it with them.
-        using var address = fixture.Client();
+        var signIns = new SignInCounter();
+        using var address = fixture.App.CreateDefaultClient(new Uri("https://localhost"), signIns);
+        address.DefaultRequestHeaders.Add("X-Forwarded-For", $"10.{Random.Shared.Next(255)}.{Random.Shared.Next(255)}.{Random.Shared.Next(1, 255)}");
         using var stranger = new BitweenClient(address, "bwt_" + new string('x', 40));
         var refusals = await Task.WhenAll(Enumerable.Range(0, 10)
             .Select(_ => Assert.ThrowsAsync<BitweenClientException>(() => stranger.Exchanges.SearchAsync())));
@@ -164,7 +191,20 @@ public async Task Says_what_Bitween_refused_and_why()
             Assert.Equal(HttpStatusCode.BadRequest, refused.Status);
             Assert.StartsWith("This token isn't valid", refused.Message);
         });
+        Assert.Equal(1, signIns.Count);
         using var neighbour = new BitweenClient(address, await TokenAsync("exchanges.view"));
         Assert.Equal(2, (await neighbour.Exchanges.SearchAsync(new ExchangeQuery { PartnerIds = [partnerId] })).TotalCount);
     }
+
+    /// <summary>Counts the calls that trade a token for a session.</summary>
+    sealed class SignInCounter : DelegatingHandler
+    {
+        public int Count;
+
+        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellation)
+        {
+            if (request.RequestUri!.AbsolutePath == "/api/accounts/servicetoken") Interlocked.Increment(ref Count);
+            return base.SendAsync(request, cancellation);
+        }
+    }
 }

docs/client-sdk.md

@@ -17,7 +17,7 @@ role, and issue it a token. The token, starting `bwt_`, is shown once.
 | To | The role needs |
 |---|---|
 | Search exchanges, read their files and retry chains, see scheduled retries, check and export | `exchanges.view` |
-| Retry, bulk retry, and run a scheduled retry now | `exchanges.operate` as well |
+| Retry, bulk retry, and run a scheduled retry now | `exchanges.operate` (without `exchanges.view` too, a retry can't answer with the new attempt's id) |
 
 The SDK trades the token for a session on its first call, and again whenever the session runs out or
 is refused, so the token is all you hand it.
@@ -94,7 +94,7 @@ Types come with it. Rows have the API's camelCase names, and times are ISO strin
 | `Exchanges.GetAsync(id)` | `exchanges.get(id)` | One exchange, or null |
 | `Exchanges.ReadFileAsync(key)` | `exchanges.readFile(key)` | A file's text, by the row's `inputKey`, `outputKey` or `responseKey` |
 | `Exchanges.GetRetryTreeAsync(id)` | `exchanges.retryTree(id)` | Every attempt in the retry chain, oldest first |
-| `Exchanges.RetryAsync(id, reset)` | `exchanges.retry(id, { reset })` | Runs it again from its input, and answers with the new attempt's id: null when the account may retry but not view, so can't look it up. `reset` uses the subscription's current settings. |
+| `Exchanges.RetryAsync(id, reset)` | `exchanges.retry(id, { reset })` | Runs it again from its input, and answers with the new attempt's id: null when it can't look that up, because the account may retry but not view or the lookup failed. The retry is made either way. `reset` uses the subscription's current settings. |
 | `Exchanges.PreviewBulkRetryAsync(selection)` | `exchanges.previewBulkRetry(selection)` | What a bulk retry would do, changing nothing |
 | `Exchanges.BulkRetryAsync(selection)` | `exchanges.bulkRetry(selection)` | Retries a selection, and says what it did |
 | `Exchanges.CheckExportAsync(selection)` | `exchanges.checkExport(selection)` | How many the export takes, and how many are older than storage keeps files |
@@ -117,7 +117,7 @@ to hold.
 | `CorrelationId` / `correlationId` | This correlation id |
 | `LatestOnly` / `latestOnly` | Only the newest attempt of each retry chain |
 | `ErrorContains` / `errorContains` | The error contains the text, ignoring case |
-| `PromotedProperties` / `promotedProperties` | By key: the value contains the text, or with no text, the key was promoted |
+| `PromotedProperties` / `promotedProperties` | By key: the value contains the text, or with no text (null or blank), the key was promoted |
 | `Filters` / `filters` | Filters in Bitween's own `Field:Rule:Value` form, for the rest. See [Exchanges](api-reference.md#exchanges). |
 
 A scheduled retry query takes `SubscriptionId`, `InformationTypeId`, `ErrorContains`, and `From` and
@@ -126,7 +126,8 @@ A scheduled retry query takes `SubscriptionId`, `InformationTypeId`, `ErrorConta
 ### Selections
 
 Bulk retry and export take a selection: exchanges by id, or every exchange a query matches, however
-many pages that is, with some left out.
+many pages that is, with some left out. A query with an empty id list is refused rather than read as no ids at all,
+which would match every exchange.
 
 ```csharp
 ExchangeSelection.Of(["a1b2...", "c3d4..."])

sdk/node-client/README.md

@@ -14,7 +14,7 @@ for (const exchange of failed.result) console.log(exchange.id, exchange.exceptio
 ```
 
 The token is a service account's (Team > Service accounts in Bitween). Keep it on your server, never
-in browser code. See [Client SDK](../../docs/client-sdk.md) for every call, queries, selections and errors.
+in browser code. See [Client SDK](https://github.com/simplify9/Bitween-api/blob/releases/r10.0/docs/client-sdk.md) for every call, queries, selections and errors.
 
 Tests: `node --test`. They run against a stand-in Bitween; the same calls against a real one are tested
 in `SW.Bitween.HttpTests`, through the .NET client.

sdk/node-client/src/index.d.ts

@@ -21,7 +21,7 @@ export interface ExchangeQuery {
   latestOnly?: boolean;
   /** The error contains this text, ignoring case. */
   errorContains?: string;
-  /** By key: the value contains the text, ignoring case, or with null, the exchange promoted the key. */
+  /** By key: the value contains the text, ignoring case, or with null or blank text, the exchange promoted the key. */
   promotedProperties?: Record<string, string | null>;
   /** Filters in Bitween's own form, `Field:Rule:Value`, for what the rest doesn't cover. */
   filters?: string[];
@@ -32,7 +32,7 @@ export type ExchangeSelection = { ids: string[] } | { matching: ExchangeQuery; e
 
 export interface Page<T> {
   result: T[];
-  /** How many match. An exchange search stops counting at 10,001, meaning more than 10,000. */
+  /** How many match, or 0 when searched with `count: false`. An exchange search stops counting at 10,001, meaning more than 10,000. */
   totalCount: number;
 }
 

sdk/node-client/src/index.js

@@ -65,7 +65,7 @@ function exchangeFilters(query = {}) {
     filters.push(values.length === 1 ? `${field}:1:${values[0]}` : `${field}:9:numeric|${values.join("|")}`);
   };
   if (query.status != null) {
-    if (!(query.status in STATUS)) throw new TypeError(`status is one of ${Object.keys(STATUS).join(", ")}.`);
+    if (!Object.hasOwn(STATUS, query.status)) throw new TypeError(`status is one of ${Object.keys(STATUS).join(", ")}.`);
     filters.push(`StatusFilter:1:${STATUS[query.status]}`);
   }
   oneOf("PartnerId", query.partnerIds);
@@ -88,8 +88,10 @@ function exchangeFilters(query = {}) {
 
 /** A selection as the bulk endpoints take it: `{ ids }`, or `{ matching, except }`. */
 function selectionBody(selection) {
-  if (selection?.ids) return { ids: [...selection.ids] };
-  if (!selection?.matching) throw new TypeError("A selection is { ids } or { matching, except }.");
+  if (!selection?.ids === !selection?.matching) throw new TypeError("A selection is { ids } or { matching, except }.");
+  if (selection.ids) return { ids: [...selection.ids] };
+  // No ids adds no filter, so it would select every exchange rather than none.
+  if (selection.matching.ids?.length === 0) throw new TypeError("matching.ids is empty: it would match every exchange.");
   const filters = exchangeFilters(selection.matching);
   // An empty filter reads as no filter, and the selection would fall back to its ids, which are
   // none. Every attempt, not only the newest, matches everything without narrowing it.
@@ -134,21 +136,20 @@ class Exchanges {
   }
 
   /**
-   * Runs an exchange again from its input file, and resolves to the new attempt's id: null when the
-   * account may retry but not view exchanges, so can't look it up. An exchange is retried at most
+   * Runs an exchange again from its input file, and resolves to the new attempt's id: null when it
+   * can't look that up, because the account may retry but not view exchanges or the lookup failed. An exchange is retried at most
    * once: one that has been is refused with ALREADY_RETRIED. One with an automatic retry booked is
    * refused with AUTO_RETRY_SCHEDULED; use scheduledRetries.runNow instead.
    */
   async retry(id, { reset = false, reason = "Manual retry" } = {}) {
     await this.#request("POST", `api/xchanges/${encodeURIComponent(id)}/retry`, { reason, reset });
     // Answered with nothing: the new attempt is the newest that says it retries this. Looking it up
-    // needs exchanges.view, and the retry is made by then, so a refusal mustn't read as the retry's.
+    // needs exchanges.view, and the retry is made by then, so a failed lookup mustn't read as the retry's.
     try {
       const retry = await this.search({ retryFor: id }, { page: 0, size: 1, count: false });
       return retry.result[0]?.id ?? null;
-    } catch (e) {
-      if (e instanceof BitweenClientError && e.status === 403) return null;
-      throw e;
+    } catch {
+      return null;
     }
   }
 

sdk/node-client/test/client.test.js

@@ -60,6 +60,7 @@ test("searches with the Exchanges page's filters, signed in once for calls made
   assert.deepStrictEqual([search.query.page, search.query.size, search.query.count], ["2", "10", "false"]);
   assert.deepStrictEqual([plain.filters, plain.query.page, plain.query.size, plain.query.count], [[], "0", "25", undefined]);
   await assert.rejects(bitween.exchanges.search({ status: "broken" }), TypeError);
+  await assert.rejects(bitween.exchanges.search({ status: "toString" }), TypeError);
 });
 
 test("signs in again once when the session is refused", async () => {
@@ -127,6 +128,9 @@ test("reads on past an old chain's retries to the exchange, and retries without
 
   // The retry is made; only looking it up is refused, so it answers with no id rather than throwing.
   assert.strictEqual(await bitween.exchanges.retry("x"), null);
+
+  const { client: failing } = await fakeBitween((r) => r.method === "POST" ? [200, null] : [503, null]);
+  assert.strictEqual(await failing().exchanges.retry("x"), null);
 });
 
 test("sends a selection as the bulk endpoints take it", async () => {
@@ -136,6 +140,9 @@ test("sends a selection as the bulk endpoints take it", async () => {
   await bitween.exchanges.previewBulkRetry({ matching: { status: "failed", partnerIds: [7] }, except: ["c"] }, { reset: true });
   await bitween.exchanges.checkExport({ matching: {} });
   await assert.rejects(bitween.exchanges.bulkRetry({}), TypeError);
+  await assert.rejects(bitween.exchanges.bulkRetry({ ids: [], matching: { status: "failed" } }), TypeError);
+  // No ids would read as no filter, and match every exchange.
+  await assert.rejects(bitween.exchanges.bulkRetry({ matching: { ids: [] } }), TypeError);
 
   const [byId, matching, everything] = seen.requests.map((r) => [r.path, r.body]);
   assert.deepStrictEqual(byId, ["/api/xchanges/bulkretry", { ids: ["a", "b"], reason: "Bulk retry", reset: false }]);