fix(core): Stop rejecting in flush and close when client undefined (#3846)

Prior to this change, if the SDK had no client defined, calling `Sentry.close()` or `Sentry.flush()` would result in a promise rejection. Since this is not an error per se, this changes that rejection to a logged warning and a promise resolution with `false`. It also fleshes out the docstrings on all of the downstream methods (all of which essentially do the same thing - wait for the event queue to be empty, or timeout if it takes too long).

Fixes #3031.

Author: lobsterkatie

packages/browser/src/sdk.ts

@@ -146,31 +146,37 @@ export function onLoad(callback: () => void): void {
 }
 
 /**
- * A promise that resolves when all current events have been sent.
- * If you provide a timeout and the queue takes longer to drain the promise returns false.
+ * Call `flush()` on the current client, if there is one. See {@link Client.flush}.
  *
- * @param timeout Maximum time in ms the client should wait.
+ * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause
+ * the client to wait until all events are sent before resolving the promise.
+ * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it
+ * doesn't (or if there's no client defined).
  */
 export function flush(timeout?: number): PromiseLike<boolean> {
   const client = getCurrentHub().getClient<BrowserClient>();
   if (client) {
     return client.flush(timeout);
   }
-  return SyncPromise.reject(false);
+  logger.warn('Cannot flush events. No client defined.');
+  return SyncPromise.resolve(false);
 }
 
 /**
- * A promise that resolves when all current events have been sent.
- * If you provide a timeout and the queue takes longer to drain the promise returns false.
+ * Call `close()` on the current client, if there is one. See {@link Client.close}.
  *
- * @param timeout Maximum time in ms the client should wait.
+ * @param timeout Maximum time in ms the client should wait to flush its event queue before shutting down. Omitting this
+ * parameter will cause the client to wait until all events are sent before disabling itself.
+ * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it
+ * doesn't (or if there's no client defined).
  */
 export function close(timeout?: number): PromiseLike<boolean> {
   const client = getCurrentHub().getClient<BrowserClient>();
   if (client) {
     return client.close(timeout);
   }
-  return SyncPromise.reject(false);
+  logger.warn('Cannot flush events and disable SDK. No client defined.');
+  return SyncPromise.resolve(false);
 }
 
 /**

packages/node/src/sdk.ts

@@ -1,7 +1,7 @@
 import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
 import { getMainCarrier, setHubOnCarrier } from '@sentry/hub';
 import { SessionStatus } from '@sentry/types';
-import { getGlobalObject } from '@sentry/utils';
+import { getGlobalObject, logger } from '@sentry/utils';
 import * as domain from 'domain';
 
 import { NodeClient } from './client';
@@ -140,31 +140,37 @@ export function lastEventId(): string | undefined {
 }
 
 /**
- * A promise that resolves when all current events have been sent.
- * If you provide a timeout and the queue takes longer to drain the promise returns false.
+ * Call `flush()` on the current client, if there is one. See {@link Client.flush}.
  *
- * @param timeout Maximum time in ms the client should wait.
+ * @param timeout Maximum time in ms the client should wait to flush its event queue. Omitting this parameter will cause
+ * the client to wait until all events are sent before resolving the promise.
+ * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it
+ * doesn't (or if there's no client defined).
  */
 export async function flush(timeout?: number): Promise<boolean> {
   const client = getCurrentHub().getClient<NodeClient>();
   if (client) {
     return client.flush(timeout);
   }
-  return Promise.reject(false);
+  logger.warn('Cannot flush events. No client defined.');
+  return Promise.resolve(false);
 }
 
 /**
- * A promise that resolves when all current events have been sent.
- * If you provide a timeout and the queue takes longer to drain the promise returns false.
+ * Call `close()` on the current client, if there is one. See {@link Client.close}.
  *
- * @param timeout Maximum time in ms the client should wait.
+ * @param timeout Maximum time in ms the client should wait to flush its event queue before shutting down. Omitting this
+ * parameter will cause the client to wait until all events are sent before disabling itself.
+ * @returns A promise which resolves to `true` if the queue successfully drains before the timeout, or `false` if it
+ * doesn't (or if there's no client defined).
  */
 export async function close(timeout?: number): Promise<boolean> {
   const client = getCurrentHub().getClient<NodeClient>();
   if (client) {
     return client.close(timeout);
   }
-  return Promise.reject(false);
+  logger.warn('Cannot flush events and disable SDK. No client defined.');
+  return Promise.resolve(false);
 }
 
 /**

packages/types/src/client.ts

@@ -60,18 +60,22 @@ export interface Client<O extends Options = Options> {
   getOptions(): O;
 
   /**
-   * A promise that resolves when all current events have been sent.
-   * If you provide a timeout and the queue takes longer to drain the promise returns false.
+   * Flush the event queue and set the client to `enabled = false`. See {@link Client.flush}.
    *
-   * @param timeout Maximum time in ms the client should wait.
+   * @param timeout Maximum time in ms the client should wait before shutting down. Omitting this parameter will cause
+   *   the client to wait until all events are sent before disabling itself.
+   * @returns A promise which resolves to `true` if the flush completes successfully before the timeout, or `false` if
+   * it doesn't.
    */
   close(timeout?: number): PromiseLike<boolean>;
 
   /**
-   * A promise that resolves when all current events have been sent.
-   * If you provide a timeout and the queue takes longer to drain the promise returns false.
+   * Wait for all events to be sent or the timeout to expire, whichever comes first.
    *
-   * @param timeout Maximum time in ms the client should wait.
+   * @param timeout Maximum time in ms the client should wait for events to be flushed. Omitting this parameter will
+   *   cause the client to wait until all events are sent before resolving the promise.
+   * @returns A promise that will resolve with `true` if all events are sent before the timeout, or `false` if there are
+   * still events in the queue when the timeout is reached.
    */
   flush(timeout?: number): PromiseLike<boolean>;
 

packages/types/src/transport.ts

@@ -21,9 +21,12 @@ export interface Transport {
   sendSession?(session: Session | SessionAggregates): PromiseLike<Response>;
 
   /**
-   * Call this function to wait until all pending requests have been sent.
+   * Wait for all events to be sent or the timeout to expire, whichever comes first.
    *
-   * @param timeout Number time in ms to wait until the buffer is drained.
+   * @param timeout Maximum time in ms the transport should wait for events to be flushed. Omitting this parameter will
+   *   cause the transport to wait until all events are sent before resolving the promise.
+   * @returns A promise that will resolve with `true` if all events are sent before the timeout, or `false` if there are
+   * still events in the queue when the timeout is reached.
    */
   close(timeout?: number): PromiseLike<boolean>;
 }