From f76a2c34d36d715c1e6ee416e2b05650235e68bb Mon Sep 17 00:00:00 2001 From: Kristaps Fabians Geikins Date: Tue, 25 Mar 2025 13:36:49 +0200 Subject: [PATCH] chore: add no floating promises lint rule (#4249) * chore: add no floating promises lint rule * minor cleanup * fix test by only running if node 22 or greater --------- Co-authored-by: Iain Sproat <68657+iainsproat@users.noreply.github.com> --- packages/server/eslint.config.mjs | 2 +- .../modules/automate/services/authCode.ts | 2 +- packages/server/modules/blobstorage/index.ts | 6 +- packages/server/modules/cli/index.ts | 2 +- .../modules/core/graph/directives/hasScope.ts | 4 +- .../modules/core/services/taskScheduler.ts | 2 +- .../integration/objectsStream.rest.spec.ts | 262 +++++++++--------- .../core/tests/integration/userEmails.spec.ts | 2 +- packages/server/modules/gatekeeper/index.ts | 4 +- .../server/modules/gendo/services/index.ts | 4 +- .../modules/multiregion/services/queue.ts | 2 +- .../modules/notifications/services/queue.ts | 2 +- packages/server/modules/previews/index.ts | 2 +- .../workspaces/tests/integration/sso.spec.ts | 2 +- .../integration/workspaces.graph.spec.ts | 2 +- .../components/knex/knexMonitoring.ts | 2 +- 16 files changed, 154 insertions(+), 148 deletions(-) diff --git a/packages/server/eslint.config.mjs b/packages/server/eslint.config.mjs index 18cb3da89..b4203b926 100644 --- a/packages/server/eslint.config.mjs +++ b/packages/server/eslint.config.mjs @@ -46,9 +46,9 @@ const configs = [ ], '@typescript-eslint/no-explicit-any': 'error', '@typescript-eslint/no-unsafe-return': 'error', + '@typescript-eslint/no-floating-promises': 'error', '@typescript-eslint/no-base-to-string': 'off', '@typescript-eslint/no-misused-promises': 'off', // breaks async middlewares (could be fixed tho) - '@typescript-eslint/no-floating-promises': 'off', // too many false positives in knex query builders '@typescript-eslint/restrict-template-expressions': 'off', // too restrictive '@typescript-eslint/no-unsafe-enum-comparison': 'off', // too restrictive '@typescript-eslint/unbound-method': 'off', // too many false positives diff --git a/packages/server/modules/automate/services/authCode.ts b/packages/server/modules/automate/services/authCode.ts index cab3950bd..ee3a7fc78 100644 --- a/packages/server/modules/automate/services/authCode.ts +++ b/packages/server/modules/automate/services/authCode.ts @@ -86,7 +86,7 @@ export const validateStoredAuthCodeFactory = // Token is valid, confirm user is authorized to access specified resources. if (resources?.workspaceId) { - emit({ + await emit({ eventName: 'workspace.authorized', payload: { userId: payload.userId, workspaceId: resources?.workspaceId } }) diff --git a/packages/server/modules/blobstorage/index.ts b/packages/server/modules/blobstorage/index.ts index efb8bb834..3d42ecdd3 100644 --- a/packages/server/modules/blobstorage/index.ts +++ b/packages/server/modules/blobstorage/index.ts @@ -304,7 +304,7 @@ export const init: SpeckleModule['init'] = async ({ app }) => { ])(req, res, next) }, async (req, res) => { - errorHandler(req, res, async (req, res) => { + await errorHandler(req, res, async (req, res) => { const streamId = req.params.streamId const [projectDb, projectStorage] = await Promise.all([ getProjectDbClient({ projectId: streamId }), @@ -339,7 +339,7 @@ export const init: SpeckleModule['init'] = async ({ app }) => { await authMiddlewareCreator(createStreamReadPermissions())(req, res, next) }, async (req, res) => { - errorHandler(req, res, async (req, res) => { + await errorHandler(req, res, async (req, res) => { const streamId = req.params.streamId const [projectDb, projectStorage] = await Promise.all([ getProjectDbClient({ projectId: streamId }), @@ -378,7 +378,7 @@ export const init: SpeckleModule['init'] = async ({ app }) => { const getBlobMetadataCollection = getBlobMetadataCollectionFactory({ db: projectDb }) - errorHandler(req, res, async (req, res) => { + await errorHandler(req, res, async (req, res) => { const blobMetadataCollection = await getBlobMetadataCollection({ streamId: req.params.streamId, query: fileName as string diff --git a/packages/server/modules/cli/index.ts b/packages/server/modules/cli/index.ts index 1198e18df..7447681cc 100644 --- a/packages/server/modules/cli/index.ts +++ b/packages/server/modules/cli/index.ts @@ -45,7 +45,7 @@ const main = async () => { return execution } -main().then(() => { +void main().then(() => { // weird TS typing issue yargs.exit(0, undefined as unknown as Error) }) diff --git a/packages/server/modules/core/graph/directives/hasScope.ts b/packages/server/modules/core/graph/directives/hasScope.ts index e47e1e0af..e2b4dcc53 100644 --- a/packages/server/modules/core/graph/directives/hasScope.ts +++ b/packages/server/modules/core/graph/directives/hasScope.ts @@ -66,9 +66,9 @@ export const hasScopes: GraphqlDirectiveBuilder = () => { const currentScopes = context.scopes await Promise.all( - requiredScopes.map(async (requiredScope: string) => { + requiredScopes.map((requiredScope: string) => validateScopes(currentScopes, requiredScope) - }) + ) ) const data = await resolve.apply(this, args) diff --git a/packages/server/modules/core/services/taskScheduler.ts b/packages/server/modules/core/services/taskScheduler.ts index 31196e63a..023382022 100644 --- a/packages/server/modules/core/services/taskScheduler.ts +++ b/packages/server/modules/core/services/taskScheduler.ts @@ -46,7 +46,7 @@ export const scheduledCallbackWrapper = async ( 'The triggered task execution {taskName} failed at {scheduledTime}' ) } finally { - releaseTaskLock(lock) + await releaseTaskLock(lock) } } diff --git a/packages/server/modules/core/tests/integration/objectsStream.rest.spec.ts b/packages/server/modules/core/tests/integration/objectsStream.rest.spec.ts index 96bbf2989..8e653bdcc 100644 --- a/packages/server/modules/core/tests/integration/objectsStream.rest.spec.ts +++ b/packages/server/modules/core/tests/integration/objectsStream.rest.spec.ts @@ -48,6 +48,8 @@ import { parse, Parser } from 'csv-parse' import { createReadStream } from 'fs' import { createObjectsBatchedAndNoClosuresFactory } from '@/modules/core/services/objects/management' +const IS_NODE_22_OR_ABOVE = process.versions.node.split('.').map(Number)[0] >= 22 + const getServerInfo = getServerInfoFactory({ db }) const getUser = legacyGetUserFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ @@ -99,144 +101,148 @@ describe('Objects streaming REST @core', () => { const ctx = await beforeEachContext() ;({ serverAddress } = await initializeTestServer(ctx)) }) - - it('should close database connections if client connection is prematurely closed', async () => { - const userId = await createUser({ - name: 'emails user', - email: createRandomEmail(), - password: createRandomPassword() - }) - const user = await getUser(userId) - - const project = { - id: '', - name: 'test project', - ownerId: userId - } - await createTestStream(project as unknown as BasicTestStream, user) - - const token = `Bearer ${await createPersonalAccessToken( - user.id, - 'test token user A', - [ - Scopes.Streams.Read, - Scopes.Streams.Write, - Scopes.Users.Read, - Scopes.Users.Email, - Scopes.Tokens.Write, - Scopes.Tokens.Read, - Scopes.Profile.Read, - Scopes.Profile.Email - ] - )}` - - const manyObjs: { commit: RawSpeckleObject; objs: RawSpeckleObject[] } = - generateManyObjects(3333, 'perlin merlin magic') - const objsIds = manyObjs.objs.map((o) => o.id) - - await createObjectsBatched({ streamId: project.id, objects: manyObjs.objs }) - for (let i = 0; i < 4; i++) { - forceCloseStreamingConnection({ - serverAddress, - projectId: project.id, - token, - objsIds + ;(IS_NODE_22_OR_ABOVE ? it : it.skip)( + 'should close database connections if client connection is prematurely closed', + async () => { + const userId = await createUser({ + name: 'emails user', + email: createRandomEmail(), + password: createRandomPassword() }) - } + const user = await getUser(userId) - //sleep for a bit to allow the server to close the connections - await new Promise((r) => setTimeout(r, 3000)) - const gaugeContents = await determineRemainingDatabaseConnectionCapacity({ - serverAddress - }) - expect(parseInt(gaugeContents), gaugeContents).to.gte(4) //expect all connections to become available again after the client closes them - }) + const project = { + id: '', + name: 'test project', + ownerId: userId + } + await createTestStream(project as unknown as BasicTestStream, user) - it('should stream model with some failing feature', async () => { - const userId = await createUser({ - name: 'emails user', - email: createRandomEmail(), - password: createRandomPassword() - }) - const user = await getUser(userId) + const token = `Bearer ${await createPersonalAccessToken( + user.id, + 'test token user A', + [ + Scopes.Streams.Read, + Scopes.Streams.Write, + Scopes.Users.Read, + Scopes.Users.Email, + Scopes.Tokens.Write, + Scopes.Tokens.Read, + Scopes.Profile.Read, + Scopes.Profile.Email + ] + )}` - const project = { - id: '', - name: 'test project', - ownerId: userId - } - await createTestStream(project as unknown as BasicTestStream, user) + const manyObjs: { commit: RawSpeckleObject; objs: RawSpeckleObject[] } = + generateManyObjects(3333, 'perlin merlin magic') + const objsIds = manyObjs.objs.map((o) => o.id) - const token = `Bearer ${await createPersonalAccessToken( - user.id, - 'test token user A', - [ - Scopes.Streams.Read, - Scopes.Streams.Write, - Scopes.Users.Read, - Scopes.Users.Email, - Scopes.Tokens.Write, - Scopes.Tokens.Read, - Scopes.Profile.Read, - Scopes.Profile.Email - ] - )}` - - // import CSV file - const csvStream = createReadStream( - //FIXME this relies on running this test from `packages/server` directory - `${process.cwd()}/test/assets/failing-streaming-model-f547dc4e88.csv` - ) - // eslint-disable-next-line camelcase - .pipe(parse({ delimiter: ',', from_line: 2 })) - - function csvParserAsPromise( - stream: Parser - ): Promise<{ manyObjs: RawSpeckleObject[]; objsIds: string[] }> { - const manyObjs: RawSpeckleObject[] = [] - const objsIds: string[] = [] - return new Promise((resolve, reject) => { - stream.on('data', (row: string[]) => { - const obj = JSON.parse(row[1]) - manyObjs.push(obj) - objsIds.push(row[0]) + await createObjectsBatched({ streamId: project.id, objects: manyObjs.objs }) + for (let i = 0; i < 4; i++) { + await forceCloseStreamingConnection({ + serverAddress, + projectId: project.id, + token, + objsIds }) - stream.on('end', () => resolve({ manyObjs, objsIds })) - // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors - stream.on('error', (error: unknown) => reject(error)) + } + + //sleep for a bit to allow the server to close the connections + await new Promise((r) => setTimeout(r, 3000)) + const gaugeContents = await determineRemainingDatabaseConnectionCapacity({ + serverAddress }) + expect(parseInt(gaugeContents), gaugeContents).to.gte(4) //expect all connections to become available again after the client closes them } - - const { manyObjs, objsIds } = await csvParserAsPromise(csvStream) - - const preGaugeContents = await determineRemainingDatabaseConnectionCapacity({ - serverAddress - }) - expect( - parseInt(preGaugeContents), - `Prior to test, we did not have sufficient DB connections free: ${preGaugeContents}` - ).to.gte(4) // all connections are available before the test - - await createObjectsBatched({ streamId: project.id, objects: manyObjs }) - for (let i = 0; i < 1; i++) { - forceCloseStreamingConnection({ - serverAddress, - projectId: project.id, - token, - objsIds + ) + ;(IS_NODE_22_OR_ABOVE ? it : it.skip)( + 'should stream model with some failing feature', + async () => { + const userId = await createUser({ + name: 'emails user', + email: createRandomEmail(), + password: createRandomPassword() }) - } + const user = await getUser(userId) - //sleep for a bit to allow the server to close the connections - await new Promise((r) => setTimeout(r, 3000)) - const postGaugeContents = await determineRemainingDatabaseConnectionCapacity({ - serverAddress - }) - expect( - parseInt(postGaugeContents), - `After the test, we did not have sufficient DB connections free: ${postGaugeContents}` - ).to.gte(4) //expect all connections to become available again after the client closes them - }).timeout(50000) + const project = { + id: '', + name: 'test project', + ownerId: userId + } + await createTestStream(project as unknown as BasicTestStream, user) + + const token = `Bearer ${await createPersonalAccessToken( + user.id, + 'test token user A', + [ + Scopes.Streams.Read, + Scopes.Streams.Write, + Scopes.Users.Read, + Scopes.Users.Email, + Scopes.Tokens.Write, + Scopes.Tokens.Read, + Scopes.Profile.Read, + Scopes.Profile.Email + ] + )}` + + // import CSV file + const csvStream = createReadStream( + //FIXME this relies on running this test from `packages/server` directory + `${process.cwd()}/test/assets/failing-streaming-model-f547dc4e88.csv` + ) + // eslint-disable-next-line camelcase + .pipe(parse({ delimiter: ',', from_line: 2 })) + + function csvParserAsPromise( + stream: Parser + ): Promise<{ manyObjs: RawSpeckleObject[]; objsIds: string[] }> { + const manyObjs: RawSpeckleObject[] = [] + const objsIds: string[] = [] + return new Promise((resolve, reject) => { + stream.on('data', (row: string[]) => { + const obj = JSON.parse(row[1]) + manyObjs.push(obj) + objsIds.push(row[0]) + }) + stream.on('end', () => resolve({ manyObjs, objsIds })) + // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors + stream.on('error', (error: unknown) => reject(error)) + }) + } + + const { manyObjs, objsIds } = await csvParserAsPromise(csvStream) + + const preGaugeContents = await determineRemainingDatabaseConnectionCapacity({ + serverAddress + }) + expect( + parseInt(preGaugeContents), + `Prior to test, we did not have sufficient DB connections free: ${preGaugeContents}` + ).to.gte(4) // all connections are available before the test + + await createObjectsBatched({ streamId: project.id, objects: manyObjs }) + for (let i = 0; i < 1; i++) { + await forceCloseStreamingConnection({ + serverAddress, + projectId: project.id, + token, + objsIds + }) + } + + //sleep for a bit to allow the server to close the connections + await new Promise((r) => setTimeout(r, 3000)) + const postGaugeContents = await determineRemainingDatabaseConnectionCapacity({ + serverAddress + }) + expect( + parseInt(postGaugeContents), + `After the test, we did not have sufficient DB connections free: ${postGaugeContents}` + ).to.gte(4) //expect all connections to become available again after the client closes them + } + ).timeout(50000) }) const forceCloseStreamingConnection = async (params: { diff --git a/packages/server/modules/core/tests/integration/userEmails.spec.ts b/packages/server/modules/core/tests/integration/userEmails.spec.ts index 96dad8cc7..c076258d2 100644 --- a/packages/server/modules/core/tests/integration/userEmails.spec.ts +++ b/packages/server/modules/core/tests/integration/userEmails.spec.ts @@ -437,7 +437,7 @@ describe('Core @user-emails', () => { assertLowercase(updatedEmail) randomCaseGuy.email = newEmail - updateEmailDirectly(newEmail) + await updateEmailDirectly(newEmail) }) it('with validateAndCreateUserEmailFactory()', async () => { diff --git a/packages/server/modules/gatekeeper/index.ts b/packages/server/modules/gatekeeper/index.ts index 88b1d1fd6..1e2bfd055 100644 --- a/packages/server/modules/gatekeeper/index.ts +++ b/packages/server/modules/gatekeeper/index.ts @@ -155,12 +155,12 @@ const scheduleWorkspaceTrialExpiry = ({ 'Workspace trial expired for {workspaceIds}.' ) await Promise.all( - expiredWorkspacePlans.map(async (plan) => { + expiredWorkspacePlans.map(async (plan) => emit({ eventName: 'gatekeeper.workspace-trial-expired', payload: { workspaceId: plan.workspaceId } }) - }) + ) ) } } diff --git a/packages/server/modules/gendo/services/index.ts b/packages/server/modules/gendo/services/index.ts index 272662e73..b74ec6c60 100644 --- a/packages/server/modules/gendo/services/index.ts +++ b/packages/server/modules/gendo/services/index.ts @@ -55,7 +55,7 @@ export const createRenderRequestFactory = id: crs({ length: 10 }) }) - deps.publish(ProjectSubscriptions.ProjectVersionGendoAIRenderCreated, { + await deps.publish(ProjectSubscriptions.ProjectVersionGendoAIRenderCreated, { projectVersionGendoAIRenderCreated: newRecord }) @@ -109,7 +109,7 @@ export const updateRenderRequestFactory = input: { ...input, updatedAt: new Date() }, id: baseRequest.id }) - deps.publish(ProjectSubscriptions.ProjectVersionGendoAIRenderUpdated, { + await deps.publish(ProjectSubscriptions.ProjectVersionGendoAIRenderUpdated, { projectVersionGendoAIRenderUpdated: record }) diff --git a/packages/server/modules/multiregion/services/queue.ts b/packages/server/modules/multiregion/services/queue.ts index 6b85fd32a..d02568d04 100644 --- a/packages/server/modules/multiregion/services/queue.ts +++ b/packages/server/modules/multiregion/services/queue.ts @@ -136,7 +136,7 @@ const isMultiregionJob = (job: Bull.Job): job is Bull.Job => { */ export const startQueue = async () => { const queue = getQueue() - queue.process(async (job) => { + void queue.process(async (job) => { if (!isMultiregionJob(job)) { throw new MultiRegionInvalidJobError() } diff --git a/packages/server/modules/notifications/services/queue.ts b/packages/server/modules/notifications/services/queue.ts index b9691c019..d2a7b1561 100644 --- a/packages/server/modules/notifications/services/queue.ts +++ b/packages/server/modules/notifications/services/queue.ts @@ -115,7 +115,7 @@ export function registerNotificationHandlers( */ export async function consumeIncomingNotifications() { const queue = getQueue() - queue.process(async (job): Promise => { + void queue.process(async (job): Promise => { let notificationType: Optional try { notificationsLogger.info('New notification received...') diff --git a/packages/server/modules/previews/index.ts b/packages/server/modules/previews/index.ts index f39727a79..01a607b07 100644 --- a/packages/server/modules/previews/index.ts +++ b/packages/server/modules/previews/index.ts @@ -127,7 +127,7 @@ export const init: SpeckleModule['init'] = ({ app, isInitial, metricsRegister }) }) app.use(previewRouter) - previewResponseQueue.process(async (payload, done) => { + void previewResponseQueue.process(async (payload, done) => { const parsedMessage = previewResultPayload.safeParse(payload.data) if (!parsedMessage.success) { logger.error( diff --git a/packages/server/modules/workspaces/tests/integration/sso.spec.ts b/packages/server/modules/workspaces/tests/integration/sso.spec.ts index a0a0d7ca7..5861e8693 100644 --- a/packages/server/modules/workspaces/tests/integration/sso.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/sso.spec.ts @@ -409,7 +409,7 @@ describe('Workspace SSO repositories', () => { }) afterEach(async () => { - truncateTables(['user_sso_sessions']) + await truncateTables(['user_sso_sessions']) }) describe('when deleting an sso provider that exists', async () => { diff --git a/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts index ff211e68c..aa3b28709 100644 --- a/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts @@ -834,7 +834,7 @@ describe('Workspaces GQL CRUD', () => { authorId: '' } - createTestCommit(testVersion, { + await createTestCommit(testVersion, { owner: testAdminUser, stream: workspaceProject }) diff --git a/packages/server/observability/components/knex/knexMonitoring.ts b/packages/server/observability/components/knex/knexMonitoring.ts index 81327e61d..e86306093 100644 --- a/packages/server/observability/components/knex/knexMonitoring.ts +++ b/packages/server/observability/components/knex/knexMonitoring.ts @@ -191,7 +191,7 @@ export const initKnexPrometheusMetrics = async (params: { // configure hooks on knex for (const dbClient of await params.getAllDbClients()) { if (initializedRegions.includes(dbClient.regionKey)) continue - initKnexPrometheusMetricsForRegionEvents({ + await initKnexPrometheusMetricsForRegionEvents({ logger: params.logger, region: dbClient.regionKey, db: dbClient.client