[Github #1091] action improvements (#1092)

* [gh-#1091] dont require id model if it is an action or not used at all * [gh-#1091] use ServiceResponse and use unique workflow id for actions * [gh-#1091] add back in workflowId * [gh-#1091] treat response correctly
NangoHQ · Oct 10, 2023 · de35f27 · de35f27
1 parent 60ded7a
commit de35f27
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 23 deletions.
diff --git a/packages/cli/lib/utils.ts b/packages/cli/lib/utils.ts
@@ -12,7 +12,7 @@ import { exec, spawn } from 'child_process';
 import promptly from 'promptly';
 import chalk from 'chalk';
 import type { NangoModel, NangoIntegrationData, NangoIntegration } from '@nangohq/shared';
-import { cloudHost, stagingHost, nangoConfigFile } from '@nangohq/shared';
+import { SyncConfigType, cloudHost, stagingHost, nangoConfigFile } from '@nangohq/shared';
 import * as dotenv from 'dotenv';
 import { init, generate, tsc } from './sync.js';
 
@@ -407,7 +407,20 @@ export function buildInterfaces(models: NangoModel, integrations: NangoIntegrati
         // we only care that models that are returned have an ID field
         // if the model is not returned from a sync script then it must be a
         // helper model that is used to build the returned models
-        if (returnedModels.includes(modelName) && !fields['id']) {
+        const syncForModel = Object.keys(integrations).find((providerConfigKey) => {
+            const syncObject = integrations[providerConfigKey] as unknown as { [key: string]: NangoIntegration };
+            const syncNames = Object.keys(syncObject);
+            for (let i = 0; i < syncNames.length; i++) {
+                const syncName = syncNames[i] as string;
+                const syncData = syncObject[syncName] as unknown as NangoIntegrationData;
+                if (syncData.returns && syncData.type !== SyncConfigType.ACTION) {
+                    return syncData.returns.includes(modelName);
+                }
+            }
+            return false;
+        });
+
+        if (returnedModels.includes(modelName) && !fields['id'] && syncForModel) {
             throw new Error(`Model "${modelName}" doesn't have an id field. This is required to be able to uniquely identify the data record.`);
         }
 

diff --git a/packages/server/lib/controllers/proxy.controller.ts b/packages/server/lib/controllers/proxy.controller.ts
@@ -717,8 +717,8 @@ See https://docs.nango.dev/guides/proxy#proxy-requests for more information.`
                     providerResponse: errorMessage.toString()
                 }),
                 params: {
-                    requestHeaders: JSON.stringify(config.headers),
-                    responseHeaders: JSON.stringify(error?.response?.headers)
+                    requestHeaders: JSON.stringify(config.headers, null, 2),
+                    responseHeaders: JSON.stringify(error?.response?.headers, null, 2)
                 }
             });
         } else {

diff --git a/packages/server/lib/controllers/sync.controller.ts b/packages/server/lib/controllers/sync.controller.ts
@@ -21,6 +21,7 @@ import {
     getProviderConfigBySyncAndAccount,
     SyncCommand,
     CommandToActivityLog,
+    ServiceResponse,
     errorManager,
     analytics,
     ErrorSourceEnum,
@@ -283,9 +284,20 @@ class SyncController {
 
             const syncClient = await SyncClient.getInstance();
 
-            const result = await syncClient?.triggerAction(connection as Connection, action_name as string, input, activityLogId as number);
+            const { success: actionSuccess, response: actionResponse } = (await syncClient?.triggerAction(
+                connection as Connection,
+                action_name as string,
+                input,
+                activityLogId as number
+            )) as ServiceResponse;
 
-            res.send(result);
+            if (!actionSuccess) {
+                res.sendStatus(400);
+                return;
+            } else {
+                res.send(actionResponse);
+                return;
+            }
         } catch (e) {
             next(e);
         }

diff --git a/packages/shared/lib/clients/sync.client.ts b/packages/shared/lib/clients/sync.client.ts
@@ -5,6 +5,7 @@ import fs from 'fs-extra';
 import type { Config as ProviderConfig } from '../models/Provider.js';
 import type { NangoIntegrationData, NangoConfig, NangoIntegration } from '../integrations/index.js';
 import { Sync, SyncStatus, SyncType, ScheduleStatus, SyncCommand, SyncWithSchedule } from '../models/Sync.js';
+import type { ServiceResponse } from '../models/Generic.js';
 import { LogActionEnum, LogLevel } from '../models/Activity.js';
 import { TASK_QUEUE } from '../constants.js';
 import {
@@ -21,9 +22,10 @@ import connectionService from '../services/connection.service.js';
 import configService from '../services/config.service.js';
 import { createSync } from '../services/sync/sync.service.js';
 import errorManager, { ErrorSourceEnum } from '../utils/error.manager.js';
+import { NangoError } from '../utils/error.js';
 import { isProd } from '../utils/utils.js';
 
-const generateActionWorkflowId = (actionName: string, connectionId: string) => `${TASK_QUEUE}.ACTION:${actionName}.${connectionId}`;
+const generateActionWorkflowId = (actionName: string, connectionId: string) => `${TASK_QUEUE}.ACTION:${actionName}.${connectionId}.${Date.now()}`;
 const generateWorkflowId = (sync: Sync, syncName: string, connectionId: string) => `${TASK_QUEUE}.${syncName}.${connectionId}-${sync.id}`;
 const generateScheduleId = (sync: Sync, syncName: string, connectionId: string) => `${TASK_QUEUE}.${syncName}.${connectionId}-schedule-${sync.id}`;
 
@@ -420,14 +422,17 @@ class SyncClient {
         }
     }
 
-    async triggerAction(connection: NangoConnection, actionName: string, input: object, activityLogId: number) {
+    async triggerAction(connection: NangoConnection, actionName: string, input: object, activityLogId: number): Promise<ServiceResponse> {
         const workflowId = generateActionWorkflowId(actionName, connection.connection_id as string);
 
         try {
             await createActivityLogMessage({
                 level: 'info',
                 activity_log_id: activityLogId as number,
                 content: `Starting action workflow ${workflowId} in the task queue: ${TASK_QUEUE}`,
+                params: {
+                    input: JSON.stringify(input, null, 2)
+                },
                 timestamp: Date.now()
             });
 
@@ -444,8 +449,35 @@ class SyncClient {
                 ]
             });
 
+            if (actionHandler.success === false) {
+                await createActivityLogMessageAndEnd({
+                    level: 'error',
+                    activity_log_id: activityLogId as number,
+                    timestamp: Date.now(),
+                    content: `The action workflow ${workflowId} did not complete successfully`
+                });
+
+                return actionHandler;
+            }
+
+            await createActivityLogMessageAndEnd({
+                level: 'info',
+                activity_log_id: activityLogId as number,
+                timestamp: Date.now(),
+                content: `The action workflow ${workflowId} was successfully run. A truncated response is: ${JSON.stringify(
+                    actionHandler.response,
+                    null,
+                    2
+                ).slice(0, 100)}`
+            });
+
+            await updateSuccessActivityLog(activityLogId as number, true);
+
             return actionHandler;
         } catch (e) {
+            const errorMessage = JSON.stringify(e, ['message', 'name'], 2);
+            const error = new NangoError('action_failure', { errorMessage });
+
             await createActivityLogMessageAndEnd({
                 level: 'error',
                 activity_log_id: activityLogId as number,
@@ -464,7 +496,7 @@ class SyncClient {
                 }
             });
 
-            return null;
+            return { success: false, error, response: null };
         }
     }
 

diff --git a/packages/shared/lib/models/Generic.ts b/packages/shared/lib/models/Generic.ts
@@ -19,7 +19,7 @@ export interface Deleted {
 
 export interface TimestampsAndDeleted extends Timestamps, Deleted {}
 
-export interface ServiceResponse<T> {
+export interface ServiceResponse<T = any> {
     success: boolean;
     error: NangoError | null;
     response: T | null;

diff --git a/packages/shared/lib/services/sync/run.service.ts b/packages/shared/lib/services/sync/run.service.ts
@@ -1,6 +1,7 @@
 import { loadLocalNangoConfig, nangoConfigFile } from '../nango-config.service.js';
 import type { NangoConnection } from '../../models/Connection.js';
 import { SyncResult, SyncType, SyncStatus, Job as SyncJob } from '../../models/Sync.js';
+import type { ServiceResponse } from '../../models/Generic.js';
 import { createActivityLogMessage, createActivityLogMessageAndEnd, updateSuccess as updateSuccessActivityLog } from '../activity/activity.service.js';
 import { addSyncConfigToJob, updateSyncJobResult, updateSyncJobStatus } from '../sync/job.service.js';
 import { getSyncConfig } from './config/config.service.js';
@@ -83,7 +84,12 @@ export default class SyncRun {
         }
     }
 
-    async run(optionalLastSyncDate?: Date | null, bypassEnvironment?: boolean, optionalSecretKey?: string, optionalHost?: string): Promise<boolean | object> {
+    async run(
+        optionalLastSyncDate?: Date | null,
+        bypassEnvironment?: boolean,
+        optionalSecretKey?: string,
+        optionalHost?: string
+    ): Promise<boolean | object | ServiceResponse> {
         if (this.debug) {
             const content = this.loadLocation ? `Looking for a local nango config at ${this.loadLocation}` : `Looking for a sync config for ${this.syncName}`;
             if (this.writeToDb) {
@@ -242,7 +248,7 @@ export default class SyncRun {
                         }.`
                     );
 
-                    return false;
+                    return this.isAction ? { success: false } : false;
                 }
 
                 if (!this.writeToDb) {
@@ -261,7 +267,7 @@ export default class SyncRun {
                         content
                     });
 
-                    return userDefinedResults;
+                    return { success: true, response: userDefinedResults };
                 }
 
                 if (userDefinedResults === undefined) {

diff --git a/packages/shared/lib/utils/error.ts b/packages/shared/lib/utils/error.ts
@@ -194,6 +194,14 @@ export class NangoError extends Error {
                 }
                 break;
 
+            case 'action_failure':
+                this.status = 400;
+                this.message = `Failed to perform the action. Please try again.`;
+                if (this.payload) {
+                    this.message += ` Please make sure this value exists in the Nango dashboard ${JSON.stringify(this.payload, null, 2)}`;
+                }
+                break;
+
             case 'missing_provider_template':
                 this.status = 400;
                 this.message = `Missing param 'provider'.`;

diff --git a/packages/webapp/src/pages/Activity.tsx b/packages/webapp/src/pages/Activity.tsx
@@ -33,30 +33,35 @@ interface Props {
 const JsonPrettyPrint: React.FC<Props> = ({ data }): ReactElement<any, any> => {
   let prettyJson = '';
   let message = '';
+  let isJson = true;
 
   try {
-    const jsonRegex = /(\[.*\])/s;
+    const jsonRegex = /({.*})|(\[.*\])/s;
     const match = (data as string)?.match(jsonRegex);
 
     if (match) {
       const json = JSON.parse(match[0]);
       prettyJson = JSON.stringify(json, null, 2);
-      message = (data as string)?.replace(jsonRegex, '').trim(); // Extract the message part
+      message = (data as string)?.replace(jsonRegex, '').trim();
     } else {
-      prettyJson = data as string;
+      try {
+        prettyJson = JSON.stringify(JSON.parse(data as string), null, 2);
+      } catch {
+        isJson = false;
+        prettyJson = data as string;
+      }
     }
 
     return (
-      <div>
+      <>
         {message && <p>{message}</p>}
-        <pre className="max-w-5xl overflow-auto whitespace-pre-wrap break-all">{prettyJson}</pre>
-      </div>
+        {isJson ? <pre className="max-w-5xl overflow-auto whitespace-pre-wrap break-all">{prettyJson}</pre> : <>{prettyJson}</>}
+      </>
     );
   } catch (e) {
-    return <span className="whitespace-normal break-all overflow-wrap">{data}</span>;
+      return <span className="whitespace-normal break-all overflow-wrap">{data?.toString()}</span>;
   }
 };
-
 export default function Activity() {
     const navigate = useNavigate();
 
@@ -175,7 +180,7 @@ export default function Activity() {
         return Object.entries(params).map(([key, value]) => (
             <div className={`max-w-5xl whitespace-normal break-all overflow-wrap ${level === 'error' ? 'text-red-500' : level === 'warn' ? 'text-orange-500' : ''}`} key={key}>
                 <span>{key}: </span>
-                <span className="max-w-5xl whitespace-normal break-all overflow-wrap">{value === null ? '' : value.toString()}</span>
+                {value === null ? '' : <JsonPrettyPrint data={value} />}
             </div>
         ));
     };
@@ -219,7 +224,7 @@ export default function Activity() {
                                 {activities.filter((activity: ActivityResponse) => typeof activity?.action === 'string').map((activity: ActivityResponse, index: number) => (
                                     <tr key={activity.id} ref={activityRefs[activity.id]}>
                                         <td
-                                            className={`mx-8 flex-col px-10 py-4 whitespace-nowrap ${
+                                            className={`mx-8 flex-col px-3 py-4 whitespace-nowrap ${
                                                 index !== -1 ? 'border-b border-border-gray' : ''
                                             } h-16`}
                                         >