UI cleanup and error handling improvements (#114)

2025-09-01 12:26:08 +08:00 · 2023-04-05 19:19:52 -04:00 · 2023-04-05 19:19:52 -04:00 · 378708a64f
commit 378708a64f
parent dcb41b8cca
19 changed files with 115 additions and 48 deletions
--- a/apps/platform/src/api.ts
+++ b/apps/platform/src/api.ts
@ -14,6 +14,32 @@ export default class Api extends Koa {
        this.proxy = process.env.NODE_ENV !== 'development'

        app.error.attach(this)
+        this.use(async (ctx, next) => {
+            try {
+                await next()
+            } catch (error: any) {
+                if (error instanceof RequestError) {
+                    ctx.status = error.statusCode ?? 400
+                    ctx.body = error
+                } else {
+                    ctx.status = 400
+                    ctx.body = process.env.NODE_ENV === 'production'
+                        ? {
+                            status: 'error',
+                            error: 'An error occurred with this request.',
+                        }
+                        : {
+                            status: 'error',
+                            error: {
+                                message: error.message,
+                                stack: error.stack,
+                            },
+                        }
+                }
+
+                ctx.app.emit('error', error, ctx)
+            }
+        })

        this.use(koaBody())
            .use(cors())
@ -22,19 +48,6 @@ export default class Api extends Koa {
                defer: true,
            }))

-        this.use(async (ctx, next) => {
-            try {
-                await next()
-            } catch (error) {
-                if (error instanceof RequestError) {
-                    ctx.status = error.statusCode ?? 400
-                    ctx.body = error
-                } else {
-                    throw error
-                }
-            }
-        })
-
        controllers(this)
    }
 }
--- a/apps/platform/src/client/EventPostJob.ts
+++ b/apps/platform/src/client/EventPostJob.ts
@ -27,8 +27,8 @@ export default class EventPostJob extends Job {
        const { anonymous_id, external_id } = event
        const user = await getUserFromClientId(project_id, { anonymous_id, external_id } as ClientIdentity)
        if (!user) {
-            logger.error({ project_id, event }, 'job:event_post:unknown-user')
-            throw new Error('job:event_post:unknown-user')
+            logger.warn({ project_id, event }, 'job:event_post:unknown-user')
+            return
        }

        // Create event for given user
--- a/apps/platform/src/error/BugSnagProvider.ts
+++ b/apps/platform/src/error/BugSnagProvider.ts
@ -25,4 +25,8 @@ export default class BugSnagProvider implements ErrorHandlingProvider {
            api.on('error', middleware.errorHandler)
        }
    }
+
+    notify(error: Error) {
+        Bugsnag.notify(error)
+    }
 }
--- a/apps/platform/src/error/ErrorHandler.ts
+++ b/apps/platform/src/error/ErrorHandler.ts
@ -23,4 +23,8 @@ export default class ErrorHandler {
    attach(api: Koa) {
        this.provider?.attach(api)
    }
+
+    notify(error: Error) {
+        this.provider?.notify(error)
+    }
 }
--- a/apps/platform/src/error/ErrorHandlerProvider.ts
+++ b/apps/platform/src/error/ErrorHandlerProvider.ts
@ -4,4 +4,5 @@ export type ErrorHandlerProviderName = 'bugsnag' | 'sentry'

 export default interface ErrorHandlerProvider {
    attach(api: Koa): void
+    notify(error: Error): void
 }
--- a/apps/platform/src/error/SentryProvider.ts
+++ b/apps/platform/src/error/SentryProvider.ts
@ -21,4 +21,8 @@ export default class SentryProvider implements ErrorHandlingProvider {
            })
        })
    }
+
+    notify(error: Error) {
+        Sentry.captureException(error)
+    }
 }
--- a/apps/platform/src/queue/Job.ts
+++ b/apps/platform/src/queue/Job.ts
@ -11,6 +11,14 @@ export interface EncodedJob {
    name: string
 }

+export class JobError extends Error {
+    data?: Record<any, any>
+    constructor(message: string, data?: Record<any, any>) {
+        super(message)
+        this.data = data
+    }
+}
+
 export default class Job implements EncodedJob {
    data: any
    options: JobOptions = {
--- a/apps/platform/src/queue/Queue.ts
+++ b/apps/platform/src/queue/Queue.ts
@ -1,3 +1,4 @@
+import App from '../app'
 import { DriverConfig } from '../config/env'
 import { logger } from '../config/logger'
 import { LoggerConfig } from '../providers/LoggerProvider'
@ -59,9 +60,9 @@ export default class Queue {
        logger.info(job, 'queue:job:started')
    }

-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    async errored(job: EncodedJob, error: Error) {
-        // TODO: Do something about failure
+    async errored(job: EncodedJob | undefined, error: Error) {
+        logger.error({ error, job }, 'queue:job:errored')
+        App.main.error.notify(error)
    }

    async completed(job: EncodedJob) {
--- a/apps/platform/src/queue/RedisQueueProvider.ts
+++ b/apps/platform/src/queue/RedisQueueProvider.ts
@ -1,7 +1,7 @@
 import { Queue as BullQueue, Worker } from 'bullmq'
 import { logger } from '../config/logger'
 import { batch } from '../utilities'
-import Job from './Job'
+import Job, { EncodedJob } from './Job'
 import Queue, { QueueTypeConfig } from './Queue'
 import QueueProvider from './QueueProvider'

@ -39,7 +39,7 @@ export default class RedisQueueProvider implements QueueProvider {
            const { name, data, opts } = this.adaptJob(job)
            await this.bull.add(name, data, opts)
        } catch (error) {
-            logger.error(error, 'sqs:error:enqueue')
+            logger.error(error, 'redis:error:enqueue')
        }
    }

@ -68,8 +68,13 @@ export default class RedisQueueProvider implements QueueProvider {
    start(): void {
        this.worker = new Worker('parcelvoy', async job => {
            await this.queue.dequeue(job.data)
-        }, { connection: this.config, concurrency: this.batchSize })
+        }, {
+            connection: this.config,
+            concurrency: this.batchSize,
+        })
+
        this.worker.on('failed', (job, error) => {
+            this.queue.errored(job?.data as EncodedJob, error)
            logger.error({ error }, 'redis:error:processing')
        })
    }
--- a/apps/platform/src/users/UserPatchJob.ts
+++ b/apps/platform/src/users/UserPatchJob.ts
@ -22,30 +22,15 @@ export default class UserPatchJob extends Job {
        return new this(data)
    }

-    static async handler({ project_id, user: { external_id, anonymous_id, data, ...fields }, options }: UserPatchTrigger) {
+    static async handler(patch: UserPatchTrigger) {

-        const identity = { external_id, anonymous_id } as ClientIdentity
-
-        // Check for existing user
-        const existing = await getUserFromClientId(project_id, identity)
-
-        // If user, update otherwise insert
-        const user = existing
-            ? await User.updateAndFetch(existing.id, {
-                data: data ? { ...existing.data, ...data } : undefined,
-                ...fields,
-            })
-            : await createUser(project_id, {
-                ...identity,
-                data,
-                ...fields,
-            })
+        const user = await this.upsert(patch)

        const {
            join_list_id,
            skip_list_updating = false,
            skip_journey_updating = false,
-        } = options ?? {}
+        } = patch.options ?? {}

        // Use updated user to check for list membership
        if (!skip_list_updating) {
@ -62,4 +47,29 @@ export default class UserPatchJob extends Job {
            await updateUsersJourneys(user)
        }
    }
+
+    static async upsert(patch: UserPatchTrigger, tries = 3): Promise<User> {
+        const { project_id, user: { external_id, anonymous_id, data, ...fields } } = patch
+        const identity = { external_id, anonymous_id } as ClientIdentity
+
+        // Check for existing user
+        const existing = await getUserFromClientId(project_id, identity)
+
+        // If user, update otherwise insert
+        try {
+            return existing
+                ? await User.updateAndFetch(existing.id, {
+                    data: data ? { ...existing.data, ...data } : undefined,
+                    ...fields,
+                })
+                : await createUser(project_id, {
+                    ...identity,
+                    data,
+                    ...fields,
+                })
+        } catch (error: any) {
+            // If there is an error (such as constraints, retry)
+            return this.upsert(patch, --tries)
+        }
+    }
 }
--- a/apps/ui/src/ui/ButtonGroup.css
+++ b/apps/ui/src/ui/ButtonGroup.css
@ -6,21 +6,21 @@

 .ui-button-group .ui-button,
 .ui-button-group .ui-select .select-button,
-.ui-button-group .ui-text-field input {
+.ui-button-group .ui-text-input input {
    box-shadow: none;
    margin: 0;
 }

 .ui-button-group .ui-button:not(:last-child),
 .ui-button-group .ui-select:not(:last-child) .select-button,
-.ui-button-group .ui-text-field:not(:last-child) input {
+.ui-button-group .ui-text-input:not(:last-child) input {
    border-top-right-radius: 0;
    border-bottom-right-radius: 0;
 }

 .ui-button-group .ui-button:not(:first-child),
 .ui-button-group .ui-select:not(:first-child) .select-button,
-.ui-button-group .ui-text-field:not(:first-child) input {
+.ui-button-group .ui-text-input:not(:first-child) input {
    border-top-left-radius: 0;
    border-bottom-left-radius: 0;
    
@ -31,6 +31,6 @@
 }

 .ui-button-group .ui-select:not(:first-child),
-.ui-button-group .ui-text-field:not(:first-child) {
+.ui-button-group .ui-text-input:not(:first-child) {
    margin-left: -1px;
 }
--- a/apps/ui/src/ui/Tile.css
+++ b/apps/ui/src/ui/Tile.css
@ -43,3 +43,9 @@
    margin: 0;
    color: var(--color-primary-soft);
 }
+
+@media only screen and (max-width: 600px) {
+    .ui-tile-grid {
+        grid-template-columns: 1fr;
+    }
+}
--- a/apps/ui/src/ui/form/EntityIdPicker.tsx
+++ b/apps/ui/src/ui/form/EntityIdPicker.tsx
@ -73,7 +73,7 @@ export function EntityIdPicker<T extends { id: number }>({
                {subtitle && <span className="label-subtitle">{subtitle}</span>}
            </Combobox.Label>
            <div className="ui-button-group">
-                <span className={clsx('ui-text-field', size ?? 'regular')} style={{ flexGrow: 1 }}>
+                <span className={clsx('ui-text-input', size ?? 'regular')} style={{ flexGrow: 1 }}>
                    <Combobox.Input
                        displayValue={(value: T) => value && displayValue(value)}
                        onChange={e => setQuery(e.target.value)}
--- a/apps/ui/src/views/campaign/CampaignPreview.css
+++ b/apps/ui/src/views/campaign/CampaignPreview.css
@ -18,6 +18,6 @@
    margin-bottom: 10px;
 }

-.user-lookup .ui-button-group .ui-text-field {
+.user-lookup .ui-button-group .ui-text-input {
    flex-grow: 1;
 }
--- a/apps/ui/src/views/journey/JourneyEditor.css
+++ b/apps/ui/src/views/journey/JourneyEditor.css
@ -125,6 +125,14 @@
    min-width: 200px;
 }

+.journey-step.entrance {
+    max-width: 400px;
+}
+
+.journey-step.flow {
+    max-width: 550px;
+}
+
 .journey-step.entrance.selected {
    border-color: var(--color-red-hard);
 }
--- a/apps/ui/src/views/journey/steps/Entrance.tsx
+++ b/apps/ui/src/views/journey/steps/Entrance.tsx
@ -29,7 +29,7 @@ export const entranceStep: JourneyStepType<EntranceConfig> = {
        return (
            <EntityIdPicker
                label="List"
-                subtitle="Users added to this list will automatically start this journey."
+                subtitle="When users are added to this list they will automatically enter this journey."
                required
                get={getList}
                search={searchLists}
--- a/apps/ui/src/views/settings/ApiKeys.tsx
+++ b/apps/ui/src/views/settings/ApiKeys.tsx
@ -86,7 +86,7 @@ export default function ProjectApiKeys() {
                open={Boolean(editing)}
                onClose={() => setEditing(null)}
            >
-                <Alert variant="plain" title="Key Value">{editing?.value}</Alert>
+                {editing?.value && <Alert variant="plain" title="Key Value">{editing?.value}</Alert>}
                {
                    editing && (
                        <FormWrapper<ProjectApiKey>
--- a/apps/ui/src/views/users/RuleBuilder.css
+++ b/apps/ui/src/views/users/RuleBuilder.css
@ -98,6 +98,7 @@
    color: var(--color-primary-soft);
 }

-.rule-inner input {
+.rule-inner input.small {
    font-weight: 500;
+    width: 100%;
 }
--- a/apps/ui/src/views/users/RuleBuilder.tsx
+++ b/apps/ui/src/views/users/RuleBuilder.tsx
@ -194,6 +194,7 @@ const RuleView = ({ rule, onChange, onDelete }: RuleParams) => {
                        name="path"
                        placeholder="User Property..."
                        value={rule?.path}
+                        hideLabel={true}
                        onChange={path => handleUpdate({ path })}
                    />
                    <OperatorSelector
@ -205,6 +206,7 @@ const RuleView = ({ rule, onChange, onDelete }: RuleParams) => {
                        type="text"
                        name="value"
                        placeholder="Value"
+                        hideLabel={true}
                        value={rule?.value?.toString()}
                        onChange={value => handleUpdate({ value })}
                    />