From e96da9d95d707dca261f71889772b74db3801adc Mon Sep 17 00:00:00 2001 From: Ted Kaemming <65315+tkaemming@users.noreply.github.com> Date: Tue, 21 Nov 2023 14:47:39 -0800 Subject: [PATCH] Remove `setError` completely. --- plugin-server/src/utils/db/error.ts | 14 ++++++++---- plugin-server/src/utils/db/sql.ts | 26 +---------------------- plugin-server/tests/helpers/sqlMock.ts | 1 - plugin-server/tests/utils/retries.test.ts | 1 - 4 files changed, 11 insertions(+), 31 deletions(-) diff --git a/plugin-server/src/utils/db/error.ts b/plugin-server/src/utils/db/error.ts index 1d05a2ba0e7ee..96924a35a9c17 100644 --- a/plugin-server/src/utils/db/error.ts +++ b/plugin-server/src/utils/db/error.ts @@ -1,8 +1,7 @@ import { PluginEvent, PostHogEvent, ProcessedPluginEvent } from '@posthog/plugin-scaffold' import { captureException } from '@sentry/node' -import { Hub, PluginConfig, PluginError } from '../../types' -import { setError } from './sql' +import { Hub, PluginConfig, PluginError, PluginLogEntrySource, PluginLogEntryType } from '../../types' export class DependencyUnavailableError extends Error { constructor(message: string, dependencyName: string, error: Error) { @@ -47,7 +46,7 @@ export async function processError( throw error } - const errorJson: PluginError = + const pluginError: PluginError = typeof error === 'string' ? { message: error, @@ -61,7 +60,14 @@ export async function processError( event: event, } - await setError(server, errorJson, pluginConfig) + await server.db.queuePluginLogEntry({ + pluginConfig, + source: PluginLogEntrySource.Plugin, + type: PluginLogEntryType.Error, + message: pluginError.stack ?? pluginError.message, + instanceId: server.instanceId, + timestamp: pluginError.time, + }) } export function cleanErrorStackTrace(stack: string | undefined): string | undefined { diff --git a/plugin-server/src/utils/db/sql.ts b/plugin-server/src/utils/db/sql.ts index c6dfb5dd1f19a..202f7f4ece5eb 100644 --- a/plugin-server/src/utils/db/sql.ts +++ b/plugin-server/src/utils/db/sql.ts @@ -1,14 +1,4 @@ -import { - Hub, - Plugin, - PluginAttachmentDB, - PluginCapabilities, - PluginConfig, - PluginConfigId, - PluginError, - PluginLogEntrySource, - PluginLogEntryType, -} from '../../types' +import { Hub, Plugin, PluginAttachmentDB, PluginCapabilities, PluginConfig, PluginConfigId } from '../../types' import { PostgresUse } from './postgres' function pluginConfigsInForceQuery(specificField?: keyof PluginConfig): string { @@ -115,20 +105,6 @@ export async function setPluginCapabilities( ) } -export async function setError(hub: Hub, pluginError: PluginError | null, pluginConfig: PluginConfig): Promise { - // TODO: pluginError can likely be required now, but need to check call sites more thoroughly to ensure that's the case. - if (pluginError) { - await hub.db.queuePluginLogEntry({ - pluginConfig, - source: PluginLogEntrySource.Plugin, - type: PluginLogEntryType.Error, - message: pluginError.stack ?? pluginError.message, - instanceId: hub.instanceId, - timestamp: pluginError.time, - }) - } -} - export async function disablePlugin(hub: Hub, pluginConfigId: PluginConfigId): Promise { await hub.db.postgres.query( PostgresUse.COMMON_WRITE, diff --git a/plugin-server/tests/helpers/sqlMock.ts b/plugin-server/tests/helpers/sqlMock.ts index 1da87be42a698..378c6bf6273e9 100644 --- a/plugin-server/tests/helpers/sqlMock.ts +++ b/plugin-server/tests/helpers/sqlMock.ts @@ -12,5 +12,4 @@ export const getPluginConfigRows = s.getPluginConfigRows as unknown as jest.Mock export const setPluginCapabilities = s.setPluginCapabilities as unknown as jest.MockedFunction< UnPromisify > -export const setError = s.setError as unknown as jest.MockedFunction> export const disablePlugin = s.disablePlugin as unknown as jest.MockedFunction> diff --git a/plugin-server/tests/utils/retries.test.ts b/plugin-server/tests/utils/retries.test.ts index 927c7b7675ceb..15193e8ad825c 100644 --- a/plugin-server/tests/utils/retries.test.ts +++ b/plugin-server/tests/utils/retries.test.ts @@ -8,7 +8,6 @@ import { PromiseManager } from '../../src/worker/vm/promise-manager' jest.useFakeTimers() jest.spyOn(global, 'setTimeout') -jest.mock('../../src/utils/db/error') // Mocking setError which we don't need in tests const mockHub: Hub = { instanceId: new UUID('F8B2F832-6639-4596-ABFC-F9664BC88E84'),