Document a closed client and correct opt-out and install ID docs

README.md

@@ -90,11 +90,12 @@ whatever the opt-outs say; passing `installIdFile:` lets the client do it only
 when reporting is on.
 
 A program that already keeps settings can store the ID there instead and pass
-it explicitly; `installId:` wins over `installIdFile:`:
+it explicitly. `installId:` wins over `installIdFile:`, unless it is null or
+blank; then the file is used, if one was given, and otherwise no ID is sent:
 
 ```csharp
 var trace = new TraceClient(url, "my-game", "1.4.0", key: key,
-                            installId: settings.UsageReportingInstallId); // null or blank: none sent
+                            installId: settings.UsageReportingInstallId);
 ```
 
 An explicit ID is trimmed; one over 255 characters throws `ArgumentException`,
@@ -115,12 +116,17 @@ client never generates one, never writes one, and sends nothing.
 |---|---|
 | **Returns immediately** | The HTTP call runs on one background thread the client owns. A game loop can report from its main thread and no frame waits on the network. |
 | **Never throws** | A server that is down, slow, or rejecting the key is a dropped report, not an exception in your program — nor is a logger callback that throws. Drops go to the optional `log` callback, otherwise nowhere. |
-| **Bounded** | At most 256 reports wait to be sent; past that, new ones are dropped. A trace server that is unreachable for a week costs a few kilobytes, not your memory. |
+| **Bounded** | At most 256 reports wait to be sent; past that, new ones are dropped. A trace server that is unreachable for a week costs those 256 reports, typically tens of kilobytes, not a queue that keeps growing. |
 | **Within the server's limits** | A report the server would reject for its size — more than 32 tags, or an application, name, tag key or tag value longer than 255 characters — is dropped (and logged) instead of sent. Null or blank tag keys and null tag values are skipped. |
 | **`Close()` / `Dispose()` drains** | Reports already queued get up to the client timeout (5 s total) to be sent before the thread stops, so a CLI that reports and exits at once does not lose its event. Still bounded: an unreachable server delays exit by at most the timeout plus half a second: at the timeout the in-flight request is cancelled, and the sending thread gets half a second to stop. What is still queued then is dropped, and the `log` callback is told how many. |
 
 Each request has a 5 s timeout.
 
+A closed client stays closed: `IsEnabled` is `false`, and `Report` sends nothing
+and logs nothing. Closing is not an opt-out, so `DisabledReason` and `InstallId`
+keep the values they had. A program that was reporting still sees a `null`
+`DisabledReason`, and `IsEnabled` is the way to ask whether reports are going out.
+
 The `log` callback runs on whichever thread hit the problem: the caller's for
 a report dropped in `Report` and for an installation ID file that fails in the
 constructor, and the client's background thread for a failed delivery and for

src/TraceClient/TraceClient.cs

@@ -131,22 +131,26 @@ public sealed class TraceClient : IDisposable
         /// <paramref name="installId"/> longer than <see cref="MaxLength"/> characters --
         /// programming errors, not runtime ones.
         /// </summary>
-        /// <param name="baseUrl">The trace server, e.g. <c>https://trace.danielstephenson.dev</c>.</param>
+        /// <param name="baseUrl">The trace server, e.g. <c>https://trace.danielstephenson.dev</c>. Only
+        /// a URL that does not parse is rejected: <c>ftp://host</c>, or <c>localhost:8080</c> with its
+        /// scheme left off, makes a client whose every report is a logged drop.</param>
         /// <param name="application">The program's name, exactly as its key was issued for. Keep it
         /// ASCII: it is also sent in the User-Agent header, which .NET 5+ refuses to send with
         /// non-ASCII characters, so every report would be dropped there.</param>
         /// <param name="version">The program's own version, trimmed. Sent as the tag <c>version</c>
         /// on every event unless the event carries its own.</param>
         /// <param name="key">The program's write key. Without one the client is a no-op.</param>
         /// <param name="enabled">The program's own opt-out. <c>false</c> yields a client that reports nothing.</param>
-        /// <param name="log">Where dropped reports are mentioned. Optional; treat as debug-level.
+        /// <param name="log">Where dropped reports and installation ID file failures are mentioned.
+        /// Optional; treat as debug-level.
         /// Called on the caller's thread or on the client's background thread, so it must be
         /// quick and thread-safe. A failed delivery's message includes the report's JSON body.</param>
         /// <param name="installId">The installation's ID, sent as the tag <c>install</c> on every
         /// event. It should be random -- e.g. a <see cref="Guid.NewGuid"/> the program stores in its
         /// own settings -- and never derived from a person, account or address. Trimmed; <c>null</c>
-        /// or blank means none. Longer than <see cref="MaxLength"/> characters throws
-        /// <see cref="ArgumentException"/>. Wins over <paramref name="installIdFile"/>.</param>
+        /// or blank means none was given, which leaves <paramref name="installIdFile"/> in charge.
+        /// Longer than <see cref="MaxLength"/> characters throws <see cref="ArgumentException"/>.
+        /// Any other value wins over <paramref name="installIdFile"/>.</param>
         /// <param name="installIdFile">A file, chosen by the program, that holds the installation's
         /// ID; read or created with <see cref="InstallIdFromFile"/>, but only when the client is
         /// enabled, so a disabled client never writes it. Optional; there is no default location.</param>
@@ -235,7 +239,9 @@ public static TraceClient Disabled()
         /// Why <see cref="Report"/> sends nothing: <c>null</c> when enabled, otherwise
         /// <see cref="ReasonEnvironment"/>, <see cref="ReasonConfig"/> or
         /// <see cref="ReasonNoKey"/>, verbatim, so a program can print
-        /// <c>"Usage reporting is off (" + reason + ")."</c>. Decided once, in the constructor.
+        /// <c>"Usage reporting is off (" + reason + ")."</c>. Decided once, in the constructor:
+        /// <see cref="Close"/> does not change it, so a closed client that was enabled still
+        /// says <c>null</c>. <see cref="IsEnabled"/> answers whether reports are going out.
         /// </summary>
         public string DisabledReason { get; private set; }
 
@@ -333,9 +339,9 @@ public bool IsEnabled
 
         /// <summary>
         /// Whether the environment asks for usage reporting to be off, via
-        /// <c>TRACE_USAGE_REPORTING=off</c> or <c>DO_NOT_TRACK=1</c>. Only the listed
-        /// values count; anything else (including empty) leaves the program's
-        /// own setting in charge.
+        /// <see cref="EnvUsageReporting"/> or <see cref="EnvDoNotTrack"/> set to one of
+        /// the values listed there, case-insensitive. Anything else (including empty), or
+        /// an environment that cannot be read, leaves the program's own setting in charge.
         /// </summary>
         public static bool EnvironmentDisables()
         {
@@ -379,8 +385,9 @@ private static bool IsYes(string value)
         /// never modified. A report the server would reject for its size -- more than
         /// <see cref="MaxTags"/> tags, or a string longer than <see cref="MaxLength"/> --
         /// is dropped here instead of being sent. A null or blank <paramref name="name"/>
-        /// sends nothing and logs nothing. A NaN or infinite <paramref name="value"/> is
-        /// left out of the event, which is still sent.
+        /// sends nothing and logs nothing, and so does any report on a disabled client or
+        /// after <see cref="Close"/>. A NaN or infinite <paramref name="value"/> is left
+        /// out of the event, which is still sent.
         /// </summary>
         public void Report(string name, double? value = null, IEnumerable<KeyValuePair<string, string>> tags = null)
         {

tests/TraceClient.Tests/TraceClientTest.cs

@@ -1260,7 +1260,8 @@ public void Close_ChangesIsEnabledButNotDisabledReasonOrInstallId()
         [Fact]
         public void Close_IsIdempotentAndReportAfterCloseIsANoOp()
         {
-            var client = new TraceClient(_server.BaseUrl, "MyCli", "1.2.3", key: "k");
+            var logged = new ConcurrentQueue<string>();
+            var client = new TraceClient(_server.BaseUrl, "MyCli", "1.2.3", key: "k", log: logged.Enqueue);
 
             client.Close();
             client.Close();
@@ -1270,6 +1271,7 @@ public void Close_IsIdempotentAndReportAfterCloseIsANoOp()
             Assert.False(client.IsEnabled);
             Thread.Sleep(100);
             Assert.Empty(_server.Received);
+            Assert.Empty(logged);
         }
     }
 }