From 2b47280bf0554b3dedca979b8622851e881dcacb Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Wed, 9 Oct 2024 14:20:56 -0400 Subject: [PATCH 1/5] check media for gifs; update error check for tenor --- server/src/core/server/app/handlers/api/tenor/index.ts | 8 ++++---- server/src/core/server/models/tenant/helpers.ts | 4 ++-- server/src/core/server/services/comments/media.ts | 3 +++ server/src/core/server/services/giphy/giphy.ts | 10 +++++----- server/src/core/server/services/tenor/tenor.ts | 2 +- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/server/src/core/server/app/handlers/api/tenor/index.ts b/server/src/core/server/app/handlers/api/tenor/index.ts index 073d5b27bf..31f3f8463e 100644 --- a/server/src/core/server/app/handlers/api/tenor/index.ts +++ b/server/src/core/server/app/handlers/api/tenor/index.ts @@ -69,7 +69,7 @@ export const tenorSearchHandler = return; } - const gifsEnabled = tenant.media?.gifs.enabled ?? false; + const gifsEnabled = tenant.media?.gifs?.enabled ?? false; if (!gifsEnabled) { res.status(200).send({ results: [], @@ -77,7 +77,7 @@ export const tenorSearchHandler = return; } - const apiKey = tenant.media?.gifs.key ?? null; + const apiKey = tenant.media?.gifs?.key ?? null; if (!apiKey || apiKey.length === 0) { res.status(200).send({ results: [], @@ -86,7 +86,7 @@ export const tenorSearchHandler = } const contentFilter = convertGiphyContentRatingToTenorLevel( - tenant.media?.gifs.maxRating + tenant.media?.gifs?.maxRating ); const url = new URL(TENOR_SEARCH_URL); @@ -133,7 +133,7 @@ export const tenorSearchHandler = } catch (e) { // Ensure that the API key doesn't get leaked to the logs by accident. if (e.message) { - e.message = e.message.replace(tenant.media?.gifs.key, "[Sensitive]"); + e.message = e.message.replace(tenant.media?.gifs?.key, "[Sensitive]"); } throw new WrappedInternalError(e as Error, "tenor search error"); } diff --git a/server/src/core/server/models/tenant/helpers.ts b/server/src/core/server/models/tenant/helpers.ts index 19d25e87d6..7bd05ae09b 100644 --- a/server/src/core/server/models/tenant/helpers.ts +++ b/server/src/core/server/models/tenant/helpers.ts @@ -119,13 +119,13 @@ export function supportsMediaType( return !!tenant.media?.youtube.enabled; case "giphy": return ( - !!tenant.media?.gifs.enabled && + !!tenant.media?.gifs?.enabled && !!tenant.media.gifs.key && tenant.media.gifs.provider === GQLGIF_MEDIA_SOURCE.GIPHY ); case "tenor": return ( - !!tenant.media?.gifs.enabled && + !!tenant.media?.gifs?.enabled && !!tenant.media.gifs.key && tenant.media.gifs.provider === GQLGIF_MEDIA_SOURCE.TENOR ); diff --git a/server/src/core/server/services/comments/media.ts b/server/src/core/server/services/comments/media.ts index 1d7983517d..62379cbece 100644 --- a/server/src/core/server/services/comments/media.ts +++ b/server/src/core/server/services/comments/media.ts @@ -77,6 +77,9 @@ async function attachTenorMedia( video: data.media_formats.mp4.url, }; } catch (err) { + if (!(err instanceof Error)) { + throw new Error("cannot attach Tenor Media"); + } throw new WrappedInternalError(err as Error, "cannot attach Tenor Media"); } } diff --git a/server/src/core/server/services/giphy/giphy.ts b/server/src/core/server/services/giphy/giphy.ts index 151da90ff2..8d0698158d 100644 --- a/server/src/core/server/services/giphy/giphy.ts +++ b/server/src/core/server/services/giphy/giphy.ts @@ -59,7 +59,7 @@ const GiphyRetrieveResponseSchema = Joi.object().keys({ export function ratingIsAllowed(rating: string, tenant: Tenant) { const compareRating = rating.toLowerCase(); - const maxRating = tenant.media?.gifs.maxRating || "g"; + const maxRating = tenant.media?.gifs?.maxRating || "g"; const compareIndex = RATINGS_ORDER.indexOf(compareRating); const maxIndex = RATINGS_ORDER.indexOf(maxRating); @@ -98,7 +98,7 @@ export async function searchGiphy( offset: string, tenant: Tenant ): Promise { - if (!supportsMediaType(tenant, "giphy") || !tenant.media.gifs.key) { + if (!supportsMediaType(tenant, "giphy") || !tenant.media?.gifs?.key) { throw new InternalError("Giphy was not enabled"); } @@ -137,12 +137,12 @@ export async function retrieveFromGiphy( tenant: Tenant, id: string ): Promise { - if (!supportsMediaType(tenant, "giphy") || !tenant.media.gifs.key) { + if (!supportsMediaType(tenant, "giphy") || !tenant.media?.gifs?.key) { throw new InternalError("Giphy was not enabled"); } const url = new URL(`${GIPHY_FETCH}/${id}`); - url.searchParams.set("api_key", tenant.media.gifs.key); + url.searchParams.set("api_key", tenant.media?.gifs?.key); try { const res = await fetch(url.toString()); @@ -158,7 +158,7 @@ export async function retrieveFromGiphy( } catch (err) { // Ensure that the API key doesn't get leaked to the logs by accident. if (err.message) { - err.message = err.message.replace(tenant.media.gifs.key, "[Sensitive]"); + err.message = err.message.replace(tenant.media?.gifs?.key, "[Sensitive]"); } // Rethrow the error. diff --git a/server/src/core/server/services/tenor/tenor.ts b/server/src/core/server/services/tenor/tenor.ts index f7e4f3cea1..07fb462e6e 100644 --- a/server/src/core/server/services/tenor/tenor.ts +++ b/server/src/core/server/services/tenor/tenor.ts @@ -28,7 +28,7 @@ export async function retrieveFromTenor( tenant: Tenant, id: string ): Promise { - if (!supportsMediaType(tenant, "tenor") || !tenant.media.gifs.key) { + if (!supportsMediaType(tenant, "tenor") || !tenant.media?.gifs?.key) { throw new InternalError("Tenor was not enabled"); } From f2b588d7397614665e482b8408a9b78bdefc6023 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Wed, 9 Oct 2024 14:21:43 -0400 Subject: [PATCH 2/5] check media for gifs; update error check for tenor --- server/src/core/server/services/comments/media.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/core/server/services/comments/media.ts b/server/src/core/server/services/comments/media.ts index 62379cbece..632e5d5898 100644 --- a/server/src/core/server/services/comments/media.ts +++ b/server/src/core/server/services/comments/media.ts @@ -80,7 +80,7 @@ async function attachTenorMedia( if (!(err instanceof Error)) { throw new Error("cannot attach Tenor Media"); } - throw new WrappedInternalError(err as Error, "cannot attach Tenor Media"); + throw new WrappedInternalError(err, "cannot attach Tenor Media"); } } From b9d55243a61740c699488cd1b004b26fe00fc48f Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Wed, 9 Oct 2024 14:31:12 -0400 Subject: [PATCH 3/5] convert non-error exception to error objects --- server/src/core/server/services/errors/sentry/sentry.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/src/core/server/services/errors/sentry/sentry.ts b/server/src/core/server/services/errors/sentry/sentry.ts index a6d92d5962..45208f9e66 100644 --- a/server/src/core/server/services/errors/sentry/sentry.ts +++ b/server/src/core/server/services/errors/sentry/sentry.ts @@ -87,7 +87,13 @@ export class SentryErrorReporter extends ErrorReporter { } // Get the original cause of the error in case that it is wrapped. - const errorToReport = getErrorToReport(err as Error); + let errorToReport = getErrorToReport(err as Error); + + if (!(errorToReport instanceof Error)) { + errorToReport = new Error( + "Non-error thrown: " + JSON.stringify(errorToReport) + ); + } // Capture and report the error to Sentry. const id = Sentry.captureException(errorToReport, context); From d4925911611334800ed397a9640fad24171e5f7f Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Wed, 9 Oct 2024 14:36:37 -0400 Subject: [PATCH 4/5] revert changes in sentry errors to try other changes first --- server/src/core/server/services/errors/sentry/sentry.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/server/src/core/server/services/errors/sentry/sentry.ts b/server/src/core/server/services/errors/sentry/sentry.ts index 45208f9e66..a6d92d5962 100644 --- a/server/src/core/server/services/errors/sentry/sentry.ts +++ b/server/src/core/server/services/errors/sentry/sentry.ts @@ -87,13 +87,7 @@ export class SentryErrorReporter extends ErrorReporter { } // Get the original cause of the error in case that it is wrapped. - let errorToReport = getErrorToReport(err as Error); - - if (!(errorToReport instanceof Error)) { - errorToReport = new Error( - "Non-error thrown: " + JSON.stringify(errorToReport) - ); - } + const errorToReport = getErrorToReport(err as Error); // Capture and report the error to Sentry. const id = Sentry.captureException(errorToReport, context); From 53dae816fcc2e9e232b3389a03cd012531426f82 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Thu, 31 Oct 2024 13:27:36 -0600 Subject: [PATCH 5/5] allow `maxPoolSize` to be configurable by env var (defaults to 50) - env is defined as `MONGODB_MAX_POOL_SIZE` --- server/src/core/server/config.ts | 6 ++++++ server/src/core/server/data/context.ts | 6 ++++-- server/src/core/server/services/mongodb/index.ts | 11 ++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/server/src/core/server/config.ts b/server/src/core/server/config.ts index 5caaeeb9a3..c9449a4990 100644 --- a/server/src/core/server/config.ts +++ b/server/src/core/server/config.ts @@ -186,6 +186,12 @@ const config = convict({ env: "MONGODB_ARCHIVE_URI", sensitive: true, }, + mongodb_max_pool_size: { + doc: "Max pool size for the MongoDB driver.", + format: Number, + default: 50, + env: "MONGODB_MAX_POOL_SIZE", + }, redis: { doc: "The Redis database to connect to.", format: "redis-uri", diff --git a/server/src/core/server/data/context.ts b/server/src/core/server/data/context.ts index d51c94ffba..2e133a103c 100644 --- a/server/src/core/server/data/context.ts +++ b/server/src/core/server/data/context.ts @@ -140,9 +140,11 @@ export function isArchivingEnabled(config: Config): boolean { export async function createMongoContext( config: Config ): Promise { + const maxPoolSize = config.get("mongodb_max_pool_size"); + // Setup MongoDB. const liveURI = config.get("mongodb"); - const live = (await createMongoDB(liveURI)).db; + const live = (await createMongoDB(liveURI, maxPoolSize)).db; // If we have an archive URI, use it, otherwise, default // to using the live database @@ -154,7 +156,7 @@ export async function createMongoContext( ) { archive = live; } else { - archive = (await createMongoDB(archiveURI)).db; + archive = (await createMongoDB(archiveURI, maxPoolSize)).db; } return new MongoContextImpl(live, archive); diff --git a/server/src/core/server/services/mongodb/index.ts b/server/src/core/server/services/mongodb/index.ts index 95628e69be..c0d2451892 100644 --- a/server/src/core/server/services/mongodb/index.ts +++ b/server/src/core/server/services/mongodb/index.ts @@ -3,7 +3,10 @@ import { Db, MongoClient } from "mongodb"; import { WrappedInternalError } from "coral-server/errors"; import logger from "coral-server/logger"; -async function createMongoClient(mongoURI: string): Promise { +async function createMongoClient( + mongoURI: string, + maxPoolSize = 50 +): Promise { try { return await MongoClient.connect(mongoURI, { // believe we don't need this since the new driver only uses @@ -11,6 +14,7 @@ async function createMongoClient(mongoURI: string): Promise { // issues with URL's and want to investigate into it. // useNewUrlParser: true, ignoreUndefined: true, + maxPoolSize, }); } catch (err) { throw new WrappedInternalError( @@ -45,10 +49,11 @@ interface CreateMongoDbResult { * @param config application configuration. */ export async function createMongoDB( - mongoURI: string + mongoURI: string, + maxPoolSize?: number ): Promise { // Connect and create a client for MongoDB. - const client = await createMongoClient(mongoURI); + const client = await createMongoClient(mongoURI, maxPoolSize); logger.info("mongodb has connected");