From 3d15fcc806064edd7c8573e053bc5098756ce2d2 Mon Sep 17 00:00:00 2001
From: Dhenain Ambroise <ambroise.dhenain@gmail.com>
Date: Wed, 23 Jun 2021 15:09:45 +0200
Subject: [PATCH] Add sendImmediately option

---
 .../core/amplitude/amplitudeServerClient.ts   | 55 +++++++++++--------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/src/modules/core/amplitude/amplitudeServerClient.ts b/src/modules/core/amplitude/amplitudeServerClient.ts
index 6025a2e4..75ae7d76 100644
--- a/src/modules/core/amplitude/amplitudeServerClient.ts
+++ b/src/modules/core/amplitude/amplitudeServerClient.ts
@@ -2,9 +2,11 @@ import { AMPLITUDE_EVENTS } from '@/modules/core/amplitude/events';
 import { GenericObject } from '@/modules/core/data/types/GenericObject';
 import { createLogger } from '@/modules/core/logging/logger';
 import { init } from '@amplitude/node';
+import { Event } from '@amplitude/types';
 import { LogLevel } from '@amplitude/types/dist/src/logger';
 import { Response } from '@amplitude/types/dist/src/response';
 import * as Sentry from '@sentry/node';
+import { Context } from '@sentry/types';
 import startsWith from 'lodash.startswith';
 import { v1 as uuid } from 'uuid'; // XXX Use v1 for uniqueness - See https://www.sohamkamani.com/blog/2016/10/05/uuid1-vs-uuid4/
 
@@ -23,18 +25,23 @@ const amplitudeServerClient = init(process.env.NEXT_PUBLIC_AMPLITUDE_API_KEY, {
  *
  * XXX Do not use it in Next.js pages, only in the API (it's not universal and won't work in the browser!).
  *
+ * XXX It isn't necessary to await the "logEvent()" call, as the event seem to be ingested correctly when not awaiting them.
+ *  But, if events aren't always processed (ingested by Amplitude) as expected, awaiting them might solve the issue.
+ *  See https://github.com/amplitude/Amplitude-Node/issues/123#issuecomment-866278069
+ *
  * @param eventName
  * @param userId
  * @param props
+ * @param sendImmediately
  *
  * @see https://developers.amplitude.com/docs/nodejs
  * @see https://www.npmjs.com/package/@amplitude/node
  */
-export const logEvent = async (eventName: AMPLITUDE_EVENTS, userId: string, props: GenericObject = {}): Promise<void> => {
+export const logEvent = async (eventName: AMPLITUDE_EVENTS, userId: string, props: GenericObject = {}, sendImmediately = true): Promise<void> => {
   try {
     logger.info(`Logging Amplitude event "${eventName}"${userId ? ` for user "${userId}"` : ''} with properties:`, props);
 
-    amplitudeServerClient.logEvent({
+    const event: Event = {
       event_type: eventName,
       // Either user_id or device_id must be set, they can't both be empty or "unknown" or "eventsIngested" will be set to 0 in the response
       // If the user_id isn't defined, then generate a dynamic/unique id for this event only
@@ -43,35 +50,39 @@ export const logEvent = async (eventName: AMPLITUDE_EVENTS, userId: string, prop
         'customer.ref': process.env.NEXT_PUBLIC_CUSTOMER_REF,
         ...props,
       },
-    })
+    };
+
+    amplitudeServerClient.logEvent(event)
       // .then((res) => logger.info('response', res))
       .catch((e) => logger.error(e));
 
-    // Send any events that are currently queued for sending.
-    // Will automatically happen on the next event loop.
-    // XXX It's necessary to await, or it might not work properly - See https://vercel.com/docs/platform/limits#streaming-responses
-    const response: Response = await amplitudeServerClient.flush();
+    if (sendImmediately) {
+      // Send any events that are currently queued for sending
+      // Will automatically happen on the next event loop
+      const response: Response = await amplitudeServerClient.flush();
 
-    // Monitor non 2XX response codes to allow for advanced debugging of edge cases
-    if (!startsWith(response?.statusCode?.toString(10), '2')) {
-      const message = `Amplitude event didn't return 200 response.`;
-      logger.error(message, response);
-      Sentry.withScope((scope) => {
-        scope.setContext('response', response);
-        Sentry.captureException(message);
-      });
-    } else {
-      // @ts-ignore
-      const eventsIngested = response?.body?.eventsIngested;
-
-      if (!eventsIngested) {
-        // See https://github.com/amplitude/Amplitude-Node/issues/123
-        const message = `Amplitude event wasn't ingested (it was sent, but not stored in Amplitude), expected value ">= 1" and got "${eventsIngested}". This usually happens when both user_id and device_id are invalid, they can't both be empty/undefined or 'unknown'!`;
+      // Monitor non 2XX response codes to allow for advanced debugging of edge cases
+      if (!startsWith(response?.statusCode?.toString(10), '2')) {
+        const message = `Amplitude event didn't return 200 response for event "${eventName}".`;
         logger.error(message, response);
         Sentry.withScope((scope) => {
+          scope.setContext('event', event as unknown as Context);
           scope.setContext('response', response);
           Sentry.captureException(message);
         });
+      } else {
+        // @ts-ignore
+        const eventsIngested = response?.body?.eventsIngested;
+
+        if (!eventsIngested) {
+          // See https://github.com/amplitude/Amplitude-Node/issues/123
+          const message = `Amplitude event wasn't ingested (it was sent, but not stored in Amplitude), expected value "eventsIngested >= 1" and got "eventsIngested=${eventsIngested}". This usually happens when both user_id and device_id are invalid, they can't both be empty/undefined or 'unknown'!`;
+          logger.error(message, response);
+          Sentry.withScope((scope) => {
+            scope.setContext('response', response);
+            Sentry.captureException(message);
+          });
+        }
       }
     }
   } catch (e) {