CaptureFeedback now returns a SentryId

CHANGELOG.md

@@ -7,6 +7,7 @@
 - `BreadcrumbLevel.Critical` has been renamed to `BreadcrumbLevel.Fatal` for consistency with the other Sentry SDKs ([#4605](https://github.com/getsentry/sentry-dotnet/pull/4605))
 - SentryOptions.IsEnvironmentUser now defaults to false on MAUI. The means the User.Name will no longer be set, by default, to the name of the device ([#4606](https://github.com/getsentry/sentry-dotnet/pull/4606))
 - Remove unnecessary files from SentryCocoaFramework before packing ([#4602](https://github.com/getsentry/sentry-dotnet/pull/4602))
+- CaptureFeedback now returns a `CaptureFeedbackResult` that can be used to determine whether the call succeeded or failed ([#4613](https://github.com/getsentry/sentry-dotnet/pull/4613))
 - ScopeExtensions.Populate is now internal ([#4611](https://github.com/getsentry/sentry-dotnet/pull/4611))
 
 ### Fixes

src/Sentry/CaptureFeedbackResult.cs

@@ -0,0 +1,95 @@
+namespace Sentry;
+
+/// <summary>
+/// The result type of the <see cref="ISentryClient.CaptureFeedback"/> method
+/// </summary>
+public struct CaptureFeedbackResult
+{
+    /// <summary>
+    /// Creates a successful feedback capture result with the specified event Id.
+    /// </summary>
+    /// <param name="eventId"></param>
+    public CaptureFeedbackResult(SentryId eventId)
+    {
+        if (eventId == SentryId.Empty)
+        {
+            throw new ArgumentException("EventId cannot be empty", nameof(eventId));
+        }
+
+        EventId = eventId;
+    }
+
+    /// <summary>
+    /// Creates a failed feedback capture result with the specified error reason.
+    /// </summary>
+    /// <param name="errorReason"></param>
+    public CaptureFeedbackResult(CaptureFeedbackErrorReason errorReason)
+    {
+        EventId = SentryId.Empty;
+        ErrorReason = errorReason;
+    }
+
+    /// <summary>
+    /// The Id of the captured feedback, if successful. <see cref="SentryId.Empty"/> if feedback capture fails.
+    /// </summary>
+    public SentryId EventId;
+
+    /// <inheritdoc cref="CaptureFeedbackErrorReason"/>
+    public CaptureFeedbackErrorReason? ErrorReason;
+
+    /// <summary>
+    /// Implicitly converts a <see cref="CaptureFeedbackErrorReason"/> to a <see cref="CaptureFeedbackResult"/>
+    /// </summary>
+    /// <param name="errorReason"></param>
+    /// <returns></returns>
+    public static implicit operator CaptureFeedbackResult(CaptureFeedbackErrorReason errorReason) => new(errorReason);
+
+    /// <summary>
+    /// Implicitly converts a non-empty <see cref="SentryId"/> to a <see cref="CaptureFeedbackResult"/>
+    /// </summary>
+    /// <returns></returns>
+    public static implicit operator CaptureFeedbackResult(SentryId eventId) => new(eventId);
+
+    /// <summary>
+    /// Returns true if feedback capture was successful, false otherwise.
+    /// </summary>
+    public bool Succeededed => ErrorReason == null;
+}
+
+/// <summary>
+/// Used to specify the reason why feedback capture failed, in the event of a failure
+/// </summary>
+public enum CaptureFeedbackErrorReason
+{
+    /// <summary>
+    /// <para>
+    /// An unknown error occurred (enable debug mode and check the logs for details).
+    /// </para>
+    /// <para>
+    /// Possible causes:
+    /// <list type="bullet">
+    ///   <item>
+    ///     <description>An exception from the configureScope callback</description>
+    ///   </item>
+    ///   <item>
+    ///     <description>A transmission error when sending the envelope</description>
+    ///   </item>
+    ///   <item>
+    ///     <description>An attempt to send feedback while the application is shutting down</description>
+    ///   </item>
+    ///   <item>
+    ///     <description>Something more mysterious...</description>
+    ///   </item>
+    /// </list>
+    /// </para>
+    /// </summary>
+    UnknownError,
+    /// <summary>
+    /// Sentry is disabled (very likely an empty DSN was provided when initialising the SDK).
+    /// </summary>
+    DisabledHub,
+    /// <summary>
+    /// The <see cref="SentryFeedback.Message"/> message is empty.
+    /// </summary>
+    EmptyMessage,
+}

src/Sentry/Extensibility/DisabledHub.cs

@@ -176,15 +176,18 @@ public bool CaptureEnvelope(Envelope envelope)
     /// <summary>
     /// No-Op.
     /// </summary>
-    public void CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope,
+        SentryHint? hint = null)
     {
+        return CaptureFeedbackErrorReason.DisabledHub;
     }
 
     /// <summary>
     /// No-Op.
     /// </summary>
-    public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
     {
+        return CaptureFeedbackErrorReason.DisabledHub;
     }
 
     /// <summary>

src/Sentry/Extensibility/HubAdapter.cs

@@ -260,15 +260,16 @@ public SentryId CaptureEvent(SentryEvent evt, Scope? scope, SentryHint? hint = n
     /// </summary>
     [DebuggerStepThrough]
     [EditorBrowsable(EditorBrowsableState.Never)]
-    public void CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope,
+        SentryHint? hint = null)
         => SentrySdk.CaptureFeedback(feedback, configureScope, hint);
 
     /// <summary>
     /// Forwards the call to <see cref="SentrySdk"/>.
     /// </summary>
     [DebuggerStepThrough]
     [EditorBrowsable(EditorBrowsableState.Never)]
-    public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
         => SentrySdk.CaptureFeedback(feedback, scope, hint);
 
     /// <summary>

src/Sentry/IHub.cs

@@ -134,6 +134,13 @@ public TransactionContext ContinueTrace(
     /// </summary>
     /// <param name="feedback">The feedback to send to Sentry.</param>
     /// <param name="configureScope">Callback method to configure the scope.</param>
-    /// <param name="hint">An optional hint providing high level context for the source of the event, including attachments</param>
-    public void CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope, SentryHint? hint = null);
+    /// <param name="hint">
+    /// An optional hint providing high-level context for the source of the event, including attachments
+    /// </param>
+    /// <returns>
+    /// A <see cref="CaptureFeedbackResult"/> that will contain the Id of the new event (if successful) or an error code
+    /// indicating the reason for failure (if feedback capture fails).
+    /// </returns>
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope,
+        SentryHint? hint = null);
 }

src/Sentry/ISentryClient.cs

@@ -34,7 +34,11 @@ public interface ISentryClient
     /// <param name="feedback">The feedback to send to Sentry.</param>
     /// <param name="scope">An optional scope to be applied to the event.</param>
     /// <param name="hint">An optional hint providing high level context for the source of the event</param>
-    public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null);
+    /// <returns>
+    /// A <see cref="CaptureFeedbackResult"/> that will contain the Id of the new event (if successful) or an error code
+    /// indicating the reason for failure (if feedback capture fails).
+    /// </returns>
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null);
 
     /// <summary>
     /// Captures a user feedback.

src/Sentry/Internal/Hub.cs

@@ -589,31 +589,33 @@ private SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Scope scope)
         }
     }
 
-    public void CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope,
+        SentryHint? hint = null)
     {
         if (!IsEnabled)
         {
-            return;
+            return CaptureFeedbackErrorReason.DisabledHub;
         }
 
         try
         {
             var clonedScope = CurrentScope.Clone();
             configureScope(clonedScope);
 
-            CaptureFeedback(feedback, clonedScope, hint);
+            return CaptureFeedback(feedback, clonedScope, hint);
         }
         catch (Exception e)
         {
             _options.LogError(e, "Failure to capture feedback");
+            return CaptureFeedbackErrorReason.UnknownError;
         }
     }
 
-    public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
     {
         if (!IsEnabled)
         {
-            return;
+            return CaptureFeedbackErrorReason.DisabledHub;
         }
 
         try
@@ -625,11 +627,12 @@ public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, Sentry
             }
 
             scope ??= CurrentScope;
-            CurrentClient.CaptureFeedback(feedback, scope, hint);
+            return CurrentClient.CaptureFeedback(feedback, scope, hint);
         }
         catch (Exception e)
         {
             _options.LogError(e, "Failure to capture feedback");
+            return CaptureFeedbackErrorReason.UnknownError;
         }
     }
 

src/Sentry/SentryClient.cs

@@ -20,6 +20,7 @@ public class SentryClient : ISentryClient, IDisposable
     private readonly ISessionManager _sessionManager;
     private readonly RandomValuesFactory _randomValuesFactory;
     private readonly Enricher _enricher;
+    private volatile int _isDisposed = 0;
 
     internal IBackgroundWorker Worker { get; }
 
@@ -85,12 +86,12 @@ public SentryId CaptureEvent(SentryEvent? @event, Scope? scope = null, SentryHin
     }
 
     /// <inheritdoc />
-    public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
+    public CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
     {
         if (string.IsNullOrEmpty(feedback.Message))
         {
             _options.LogWarning("Feedback dropped due to empty message.");
-            return;
+            return CaptureFeedbackErrorReason.EmptyMessage;
         }
 
         scope ??= new Scope(_options);
@@ -116,7 +117,7 @@ public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, Sentry
 
         var attachments = hint.Attachments.ToList();
         var envelope = Envelope.FromFeedback(evt, _options.DiagnosticLogger, attachments, scope.SessionUpdate);
-        CaptureEnvelope(envelope);
+        return CaptureEnvelope(envelope) ? evt.EventId : CaptureFeedbackErrorReason.UnknownError;
     }
 
     /// <inheritdoc />
@@ -438,6 +439,12 @@ private SentryId DoSendEvent(SentryEvent @event, SentryHint? hint, Scope? scope)
     /// <inheritdoc cref="ISentryClient.CaptureEnvelope"/>
     public bool CaptureEnvelope(Envelope envelope)
     {
+        if (_isDisposed == 1)
+        {
+            _options.LogWarning("Enqueue envelope failed. Client is disposed.");
+            return false;
+        }
+
         if (Worker.EnqueueEnvelope(envelope))
         {
             _options.LogInfo("Envelope queued up: '{0}'", envelope.TryGetEventId(_options.DiagnosticLogger));
@@ -456,6 +463,11 @@ public bool CaptureEnvelope(Envelope envelope)
     /// </summary>
     public void Dispose()
     {
+        if (Interlocked.Exchange(ref _isDisposed, 1) == 1)
+        {
+            return;
+        }
+
         _options.LogDebug("Flushing SentryClient.");
 
         try

src/Sentry/SentrySdk.cs

@@ -546,14 +546,14 @@ public static SentryId CaptureMessage(string message, Action<Scope> configureSco
     /// Captures feedback from the user.
     /// </summary>
     [DebuggerStepThrough]
-    public static void CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope, SentryHint? hint = null)
+    public static CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Action<Scope> configureScope, SentryHint? hint = null)
         => CurrentHub.CaptureFeedback(feedback, configureScope, hint);
 
     /// <summary>
     /// Captures feedback from the user.
     /// </summary>
     [DebuggerStepThrough]
-    public static void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
+    public static CaptureFeedbackResult CaptureFeedback(SentryFeedback feedback, Scope? scope = null, SentryHint? hint = null)
         => CurrentHub.CaptureFeedback(feedback, scope, hint);
 
     /// <summary>