fix: post script connection error + handle in logs (#2259)

## Describe your changes Fixes NAN-1093 I haven't paid enough attention when reviewing, there was a few things not working in post connection and in logs PSC: - pass missing `connection_id`, ~and missing `input` (@TBonnin I didn't wanted to change the default for input and I'm sure you already touching it in your own PR)~ Logs: - Handle post connection as it was meant to (in operation `auth`) - Create a log also for internal post connection for consistency - Properly append error to the log (do not json stringify) - UI: Show message correctly on operation hover ## Test <img width="815" alt="Screenshot 2024-06-04 at 18 17 34" src="https://github.com/NangoHQ/nango/assets/1637651/d84cd8b7-59fb-4c81-9ecc-a19203b783ef"> - @TBonnin those scripts does not work on my setup, maybe it's something on my machine but they just timeout <img width="1047" alt="Screenshot 2024-06-04 at 18 19 11" src="https://github.com/NangoHQ/nango/assets/1637651/900c262e-3a94-48af-8406-a30efadad2e9">
NangoHQ · Jun 5, 2024 · f5a9adc · f5a9adc
1 parent 186f44c
commit f5a9adc
Show file tree

Hide file tree

Showing 13 changed files with 83 additions and 80 deletions.
diff --git a/packages/orchestrator/lib/clients/types.ts b/packages/orchestrator/lib/clients/types.ts
@@ -47,6 +47,7 @@ interface PostConnectionArgs {
     postConnectionName: string;
     connection: {
         id: number;
+        connection_id: string;
         provider_config_key: string;
         environment_id: number;
     };

diff --git a/packages/server/lib/controllers/apiAuth.controller.ts b/packages/server/lib/controllers/apiAuth.controller.ts
@@ -56,7 +56,7 @@ class ApiAuthController {
                 {
                     id: String(activityLogId),
                     operation: { type: 'auth', action: 'create_connection' },
-                    message: 'Authorization API Key',
+                    message: 'Create connection via API Key',
                     expiresAt: defaultOperationExpiration.auth()
                 },
                 { account, environment }
@@ -299,7 +299,7 @@ class ApiAuthController {
                 {
                     id: String(activityLogId),
                     operation: { type: 'auth', action: 'create_connection' },
-                    message: 'Authorization Basic',
+                    message: 'Create connection via Basic Auth',
                     expiresAt: defaultOperationExpiration.auth()
                 },
                 { account, environment }

diff --git a/packages/server/lib/controllers/appStoreAuth.controller.ts b/packages/server/lib/controllers/appStoreAuth.controller.ts
@@ -49,7 +49,7 @@ class AppStoreAuthController {
                 {
                     id: String(activityLogId),
                     operation: { type: 'auth', action: 'create_connection' },
-                    message: 'Authorization App Store',
+                    message: 'Create connection via App Store',
                     expiresAt: defaultOperationExpiration.auth()
                 },
                 { account, environment }

diff --git a/packages/server/lib/controllers/oauth.controller.ts b/packages/server/lib/controllers/oauth.controller.ts
@@ -89,7 +89,7 @@ class OAuthController {
                 {
                     id: String(activityLogId),
                     operation: { type: 'auth', action: 'create_connection' },
-                    message: 'Authorization OAuth',
+                    message: 'Create connection via OAuth',
                     expiresAt: defaultOperationExpiration.auth()
                 },
                 { account, environment }
@@ -413,7 +413,7 @@ class OAuthController {
                 {
                     id: String(activityLogId),
                     operation: { type: 'auth', action: 'create_connection' },
-                    message: 'Authorization OAuth2 CC',
+                    message: 'Create connection via OAuth2 CC',
                     expiresAt: defaultOperationExpiration.auth()
                 },
                 { account, environment }

diff --git a/packages/server/lib/controllers/proxy.controller.ts b/packages/server/lib/controllers/proxy.controller.ts
@@ -126,7 +126,7 @@ class ProxyController {
             });
 
             if (credentialResponse.isErr()) {
-                await logCtx.error('Failed to get connection credentials', { error: credentialResponse.error.message });
+                await logCtx.error('Failed to get connection credentials', { error: credentialResponse.error });
                 await logCtx.failed();
                 throw new Error(`Failed to get connection credentials: '${credentialResponse.error.message}'`);
             }

diff --git a/packages/server/lib/controllers/unauth.controller.ts b/packages/server/lib/controllers/unauth.controller.ts
@@ -46,7 +46,7 @@ class UnAuthController {
 
         try {
             logCtx = await logContextGetter.create(
-                { id: String(activityLogId), operation: { type: 'auth', action: 'create_connection' }, message: 'Authorization Unauthenticated' },
+                { id: String(activityLogId), operation: { type: 'auth', action: 'create_connection' }, message: 'Create connection via Unauthenticated' },
                 { account, environment }
             );
             void analytics.track(AnalyticsTypes.PRE_UNAUTH, account.id);

diff --git a/packages/server/lib/hooks/connection/external-post-connection.ts b/packages/server/lib/hooks/connection/external-post-connection.ts
@@ -41,7 +41,7 @@ export async function externalPostConnection(
     const activityLogId = await createActivityLog(log);
 
     const logCtx = await logContextGetter.create(
-        { id: String(activityLogId), operation: { type: 'post-connection-script' }, message: 'Start action' },
+        { id: String(activityLogId), operation: { type: 'auth', action: 'post_connection' }, message: 'Start external post connection script' },
         {
             account,
             environment,

diff --git a/packages/server/lib/hooks/connection/post-connection.ts b/packages/server/lib/hooks/connection/post-connection.ts
@@ -1,8 +1,8 @@
 import type { AxiosError, AxiosResponse } from 'axios';
-import type { RecentlyCreatedConnection, Connection, ConnectionConfig, LogLevel, HTTP_VERB, UserProvidedProxyConfiguration } from '@nangohq/shared';
-import { LogActionEnum, LogTypes, createActivityLogAndLogMessage, proxyService, connectionService, telemetry } from '@nangohq/shared';
+import type { RecentlyCreatedConnection, Connection, ConnectionConfig, HTTP_VERB, UserProvidedProxyConfiguration } from '@nangohq/shared';
+import { LogActionEnum, LogTypes, proxyService, connectionService, telemetry } from '@nangohq/shared';
 import * as postConnectionHandlers from './index.js';
-import type { LogContextGetter } from '@nangohq/logs';
+import type { LogContext, LogContextGetter } from '@nangohq/logs';
 import { stringifyError } from '@nangohq/utils';
 
 type PostConnectionHandler = (internalNango: InternalNango) => Promise<void>;
@@ -19,6 +19,7 @@ export interface InternalNango {
 
 async function execute(createdConnection: RecentlyCreatedConnection, provider: string, logContextGetter: LogContextGetter) {
     const { connection: upsertedConnection, environment, account } = createdConnection;
+    let logCtx: LogContext | undefined;
     try {
         const credentialResponse = await connectionService.getConnectionCredentials({
             account,
@@ -74,8 +75,20 @@ async function execute(createdConnection: RecentlyCreatedConnection, provider: s
         const handler = handlers[`${provider.replace(/-/g, '')}PostConnection`];
 
         if (handler) {
+            logCtx = await logContextGetter.create(
+                { operation: { type: 'auth', action: 'post_connection' }, message: 'Start internal post connection script' },
+                {
+                    account,
+                    environment,
+                    integration: { id: upsertedConnection.config_id!, name: upsertedConnection.provider_config_key, provider },
+                    connection: { id: upsertedConnection.id!, name: upsertedConnection.connection_id }
+                }
+            );
+
             try {
                 await handler(internalNango);
+                await logCtx.info('Success');
+                await logCtx.success();
             } catch (e) {
                 const errorDetails =
                     e instanceof Error
@@ -87,34 +100,7 @@ async function execute(createdConnection: RecentlyCreatedConnection, provider: s
                         : 'Unknown error';
 
                 const errorString = JSON.stringify(errorDetails);
-                const log = {
-                    level: 'error' as LogLevel,
-                    success: false,
-                    action: LogActionEnum.AUTH,
-                    start: Date.now(),
-                    end: Date.now(),
-                    timestamp: Date.now(),
-                    connection_id: upsertedConnection.connection_id,
-                    provider,
-                    provider_config_key: upsertedConnection.provider_config_key,
-                    environment_id: environment.id
-                };
-
-                const activityLogId = await createActivityLogAndLogMessage(log, {
-                    level: 'error',
-                    environment_id: environment.id,
-                    timestamp: Date.now(),
-                    content: `Post connection script failed with the error: ${errorString}`
-                });
-                const logCtx = await logContextGetter.create(
-                    { id: String(activityLogId), operation: { type: 'auth', action: 'post_connection' }, message: 'Authentication' },
-                    {
-                        account,
-                        environment,
-                        integration: { id: connection.config_id!, name: connection.provider_config_key, provider },
-                        connection: { id: connection.id!, name: connection.connection_id }
-                    }
-                );
+
                 await logCtx.error('Post connection script failed', { error: e });
                 await logCtx.failed();
 
@@ -135,6 +121,9 @@ async function execute(createdConnection: RecentlyCreatedConnection, provider: s
             provider: provider,
             level: 'error'
         });
+
+        await logCtx?.error('Post connection script failed', { error: err });
+        await logCtx?.failed();
     }
 }
 

diff --git a/packages/shared/lib/clients/orchestrator.ts b/packages/shared/lib/clients/orchestrator.ts
@@ -156,7 +156,7 @@ export class Orchestrator {
                         // Errors received from temporal are raw objects not classes
                         const error = new NangoError(rawError['type'], rawError['payload'], rawError['status']);
                         res = Err(error);
-                        await logCtx.error(`Failed with error ${rawError['type']} ${JSON.stringify(rawError['payload'])}`);
+                        await logCtx.error(`Failed with error ${rawError['type']}`, { payload: rawError['payload'] });
                     } else {
                         res = Ok(response);
                     }
@@ -179,6 +179,7 @@ export class Orchestrator {
                     timestamp: Date.now(),
                     content: `Failed with error ${res.error.type} ${JSON.stringify(res.error.payload)}`
                 });
+                await logCtx.error(`Failed with error ${res.error.type}`, { payload: res.error.payload });
                 await createActivityLogMessageAndEnd({
                     level: 'error',
                     environment_id,
@@ -527,6 +528,7 @@ export class Orchestrator {
                         postConnectionName: name,
                         connection: {
                             id: connection.id!,
+                            connection_id: connection.connection_id,
                             provider_config_key: connection.provider_config_key,
                             environment_id: connection.environment_id
                         },
@@ -578,7 +580,7 @@ export class Orchestrator {
                         // Errors received from temporal are raw objects not classes
                         const error = new NangoError(rawError['type'], rawError['payload'], rawError['status']);
                         res = Err(error);
-                        await logCtx.error(`Failed with error ${rawError['type']} ${JSON.stringify(rawError['payload'])}`);
+                        await logCtx.error(`Failed with error ${rawError['type']}`, { payload: rawError['payload'] });
                     } else {
                         res = Ok(response);
                     }
@@ -601,7 +603,7 @@ export class Orchestrator {
                     timestamp: Date.now(),
                     content: `Failed with error ${res.error.type} ${JSON.stringify(res.error.payload)}`
                 });
-                await logCtx.error(`Failed with error ${res.error.type} ${JSON.stringify(res.error.payload)}`);
+                await logCtx.error(`Failed with error ${res.error.type}`, { payload: res.error.payload });
                 await createActivityLogMessageAndEnd({
                     level: 'error',
                     environment_id: connection.environment_id,
@@ -610,6 +612,7 @@ export class Orchestrator {
                     content: `The post connection script workflow ${workflowId} did not complete successfully`
                 });
                 await logCtx.error(`The post connection script workflow ${workflowId} did not complete successfully`);
+                await logCtx.failed();
 
                 return res;
             }
@@ -625,6 +628,7 @@ export class Orchestrator {
             });
             await updateSuccessActivityLog(activityLogId, true);
             await logCtx.info(content);
+            await logCtx.success();
 
             await telemetry.log(
                 LogTypes.POST_CONNECTION_SCRIPT_SUCCESS,
@@ -654,6 +658,7 @@ export class Orchestrator {
                 content
             });
             await logCtx.error(content);
+            await logCtx.failed();
 
             errorManager.report(err, {
                 source: ErrorSourceEnum.PLATFORM,

diff --git a/packages/types/lib/logs/messages.ts b/packages/types/lib/logs/messages.ts
@@ -39,9 +39,6 @@ export interface MessageOpProxy {
 export interface MessageOpAction {
     type: 'action';
 }
-export interface MessageOpPostConnectionScript {
-    type: 'post-connection-script';
-}
 export interface MessageOpAuth {
     type: 'auth';
     action: 'create_connection' | 'refresh_token' | 'post_connection';
@@ -58,15 +55,7 @@ export interface MessageOpDeploy {
     type: 'deploy';
     action: 'prebuilt' | 'custom';
 }
-export type MessageOperation =
-    | MessageOpSync
-    | MessageOpProxy
-    | MessageOpAction
-    | MessageOpPostConnectionScript
-    | MessageOpWebhook
-    | MessageOpDeploy
-    | MessageOpAuth
-    | MessageOpAdmin;
+export type MessageOperation = MessageOpSync | MessageOpProxy | MessageOpAction | MessageOpWebhook | MessageOpDeploy | MessageOpAuth | MessageOpAdmin;
 
 /**
  * Full schema

diff --git a/packages/webapp/src/pages/Logs/ShowOperation.tsx b/packages/webapp/src/pages/Logs/ShowOperation.tsx
@@ -102,7 +102,7 @@ export const ShowOperation: React.FC<{ operationId: string }> = ({ operationId }
                 <div className="flex gap-2 items-center w-[30%]">
                     <div className="font-semibold text-sm">Type</div>
                     <div className="text-gray-400 text-xs pt-[1px]">
-                        <OperationTag operation={operation.operation!} highlight />
+                        <OperationTag message={operation.message} operation={operation.operation!} highlight />
                     </div>
                 </div>
             </div>

diff --git a/packages/webapp/src/pages/Logs/components/OperationTag.tsx b/packages/webapp/src/pages/Logs/components/OperationTag.tsx
@@ -2,18 +2,32 @@
 import type { SearchOperationsData } from '@nangohq/types';
 import { cn } from '../../../utils/utils';
 import { Tag } from '../../../components/ui/label/Tag';
-import { CrossCircledIcon, Crosshair1Icon, PauseIcon, PlayIcon, ResumeIcon, UploadIcon } from '@radix-ui/react-icons';
+import {
+    CrossCircledIcon,
+    Crosshair1Icon,
+    MixerHorizontalIcon,
+    PauseIcon,
+    PersonIcon,
+    PlayIcon,
+    ReloadIcon,
+    ResumeIcon,
+    UploadIcon
+} from '@radix-ui/react-icons';
 import * as Tooltip from '../../../components/ui/Tooltip';
 
-export const OperationTag: React.FC<{ operation: Exclude<SearchOperationsData['operation'], null>; highlight?: boolean }> = ({ operation, highlight }) => {
-    if (operation.type === 'sync') {
-        return (
-            <div className="flex gap-2 items-center">
-                <Tag bgClassName={cn('bg-zinc-900', highlight && 'bg-pure-black')} textClassName={cn(highlight && 'text-white')}>
-                    {operation.type}
-                </Tag>
-                <Tooltip.Tooltip>
-                    <Tooltip.TooltipTrigger>
+export const OperationTag: React.FC<{ message: string; operation: Exclude<SearchOperationsData['operation'], null>; highlight?: boolean }> = ({
+    message,
+    operation,
+    highlight
+}) => {
+    return (
+        <Tooltip.Tooltip delayDuration={0}>
+            <Tooltip.TooltipTrigger>
+                <div className="flex items-center gap-1">
+                    <Tag bgClassName={cn('bg-zinc-900', highlight && 'bg-pure-black')} textClassName={cn(highlight && 'text-white')}>
+                        {operation.type}
+                    </Tag>
+                    {operation.type === 'sync' && (
                         <Tag bgClassName={cn('bg-zinc-900 rounded-full py-0.5', highlight && 'bg-pure-black')} textClassName={cn(highlight && 'text-white')}>
                             {operation.action === 'cancel' && <CrossCircledIcon className="w-3.5" />}
                             {operation.action === 'init' && <UploadIcon className="w-3.5" />}
@@ -23,20 +37,25 @@ export const OperationTag: React.FC<{ operation: Exclude<SearchOperationsData['o
                             {operation.action === 'unpause' && <ResumeIcon className="w-3.5" />}
                             {operation.action === 'run' && <PlayIcon className="w-3.5" />}
                         </Tag>
-                    </Tooltip.TooltipTrigger>
-                    <Tooltip.TooltipContent>
-                        <p>
-                            {operation.action} {operation.type}
-                        </p>
-                    </Tooltip.TooltipContent>
-                </Tooltip.Tooltip>
-            </div>
-        );
-    }
+                    )}
 
-    return (
-        <Tag bgClassName={cn('bg-zinc-900', highlight && 'bg-neutral-700')} textClassName={cn(highlight && 'text-white')}>
-            {operation.type}
-        </Tag>
+                    {operation.type === 'auth' && (
+                        <Tag bgClassName={cn('bg-zinc-900 rounded-full py-0.5', highlight && 'bg-pure-black')} textClassName={cn(highlight && 'text-white')}>
+                            {operation.action === 'create_connection' && <PersonIcon className="w-3.5" />}
+                            {operation.action === 'post_connection' && <MixerHorizontalIcon className="w-3.5" />}
+                            {operation.action === 'refresh_token' && <ReloadIcon className="w-3.5" />}
+                        </Tag>
+                    )}
+                </div>
+            </Tooltip.TooltipTrigger>
+            <Tooltip.TooltipContent align="start">
+                <p>
+                    {message}{' '}
+                    <code className="text-xs">
+                        ({operation.type}:{'action' in operation && operation.action})
+                    </code>
+                </p>
+            </Tooltip.TooltipContent>
+        </Tooltip.Tooltip>
     );
 };
diff --git a/packages/webapp/src/pages/Logs/constants.tsx b/packages/webapp/src/pages/Logs/constants.tsx
@@ -36,7 +36,7 @@ export const columns: ColumnDef<SearchOperationsData>[] = [
         header: 'Type',
         size: 100,
         cell: ({ row }) => {
-            return <OperationTag operation={row.original.operation!} />;
+            return <OperationTag message={row.original.message} operation={row.original.operation!} />;
         }
     },
     {