Merge pull request #2616 from omnivore-app/fix/thumbnail-delay

fix thumbnail not pre-cached if it was already set

packages/api/src/elastic/pages.ts

@@ -27,6 +27,9 @@ import {
   SearchResponse,
 } from './types'
 
+const MAX_CONTENT_LENGTH = 10 * 1024 * 1024 // 10MB
+const CONTENT_LENGTH_ERROR = 'Your page content is too large to be saved.'
+
 const appendQuery = (builder: ESBuilder, query: string): ESBuilder => {
   interface Field {
     field: string
@@ -404,6 +407,16 @@ export const createPage = async (
   ctx: PageContext
 ): Promise<string | undefined> => {
   try {
+    // max 10MB
+    if (page.content.length > MAX_CONTENT_LENGTH) {
+      logger.info('page content is too large', {
+        pageId: page.id,
+        contentLength: page.content.length,
+      })
+
+      page.content = CONTENT_LENGTH_ERROR
+    }
+
     const { body } = await client.index({
       id: page.id || undefined,
       index: INDEX_ALIAS,
@@ -432,6 +445,15 @@ export const updatePage = async (
   ctx: PageContext
 ): Promise<boolean> => {
   try {
+    if (page.content && page.content.length > MAX_CONTENT_LENGTH) {
+      logger.info('page content is too large', {
+        pageId: page.id,
+        contentLength: page.content.length,
+      })
+
+      page.content = CONTENT_LENGTH_ERROR
+    }
+
     await client.update({
       index: INDEX_ALIAS,
       id,
@@ -519,6 +541,7 @@ export const getPageByParam = async <K extends keyof ParamSet>(
     const { body } = await client.search<SearchResponse<Page>>({
       index: INDEX_ALIAS,
       body: builder.build(),
+      track_total_hits: true,
     })
 
     if (body.hits.total.value === 0) {

packages/api/src/resolvers/save/index.ts

@@ -41,7 +41,7 @@ export const savePageResolver = authorized<
   }
 
   return savePage(
-    { ...ctx, uid },
+    { ...ctx, uid, refresh: true },
     { userId: user.id, username: user.profile.username },
     input
   )

packages/api/src/routers/svc/integrations.ts

@@ -145,12 +145,11 @@ export function integrationsServiceRouter() {
 
           const synced = await integrationService.export(integration, pages)
           if (!synced) {
-            logger.info('failed to sync pages', {
+            logger.error('failed to sync pages', {
               pageIds,
               integrationId: integration.id,
             })
-            res.status(400).send('Failed to sync')
-            return
+            return res.status(400).send('Failed to sync')
           }
         }
         // delete task name if completed

packages/api/src/routers/svc/newsletters.ts

@@ -160,7 +160,7 @@ export function newsletterServiceRouter() {
 
       res.status(200).send('newsletter created')
     } catch (e) {
-      logger.info(e)
+      logger.error(e)
       if (e instanceof SyntaxError) {
         // when message is not a valid json string
         res.status(400).send(e)

packages/api/src/routers/svc/webhooks.ts

@@ -28,7 +28,7 @@ export function webhooksServiceRouter() {
 
     try {
       const data = JSON.parse(msgStr)
-      const { userId, type } = data
+      const { userId, type } = data as { userId: string; type: string }
       if (!userId || !type) {
         logger.info('No userId or type found in message')
         res.status(400).send('Bad Request')
@@ -36,7 +36,7 @@ export function webhooksServiceRouter() {
       }
 
       // example: PAGE_CREATED
-      const eventType = `${type as string}_${req.params.action}`.toUpperCase()
+      const eventType = `${type}_${req.params.action}`.toUpperCase()
       const webhooks = await getRepository(Webhook)
         .createQueryBuilder()
         .where('user_id = :userId', { userId })
@@ -46,47 +46,54 @@ export function webhooksServiceRouter() {
 
       if (webhooks.length <= 0) {
         logger.info(
-          'No active webhook found for user',
-          userId,
-          'and eventType',
-          eventType
+          'No active webhook found for user ' +
+            userId +
+            ' and eventType ' +
+            eventType
         )
         res.status(200).send('No webhook found')
         return
       }
 
       // trigger webhooks
-      for (const webhook of webhooks) {
-        const url = webhook.url
-        const method = webhook.method as Method
-        const body = JSON.stringify({
-          action: req.params.action,
-          userId,
-          [type]: data,
-        })
-
-        logger.info('triggering webhook', url)
-        try {
-          await axios.request({
-            url,
-            method,
-            headers: {
-              'Content-Type': webhook.contentType,
-            },
-            data: body,
-          })
-        } catch (error) {
-          if (axios.isAxiosError(error)) {
-            logger.error(error.response)
-          } else {
-            logger.error(error)
+      await Promise.all(
+        webhooks.map((webhook) => {
+          const url = webhook.url
+          const method = webhook.method as Method
+          const body = {
+            action: req.params.action,
+            userId,
+            [type]: data,
           }
-        }
-      }
+
+          logger.info('triggering webhook', { url, method })
+
+          return axios
+            .request({
+              url,
+              method,
+              headers: {
+                'Content-Type': webhook.contentType,
+              },
+              data: body,
+              timeout: 10000, // 10s
+            })
+            .then((response) => {
+              logger.info('webhook triggered', response.data)
+            })
+            .catch((error) => {
+              if (axios.isAxiosError(error)) {
+                logger.info('webhook failed', error.response)
+              } else {
+                logger.info('webhook failed', error)
+              }
+            })
+        })
+      )
 
       res.status(200).send('OK')
     } catch (err) {
-      logger.info('trigger webhook failed', err)
+      logger.error('trigger webhook failed', err)
       res.status(500).send(err)
     }
   })

packages/api/src/server.ts

@@ -210,8 +210,6 @@ const main = async (): Promise<void> => {
     logger.notice(`🚀 Server ready at ${apollo.graphqlPath}`)
   })
 
-  listener.timeout = 1000 * 60 * 10 // 10 minutes
-
   // Avoid keepalive timeout-related connection drops manifesting in user-facing 502s.
   // See here: https://cloud.google.com/load-balancing/docs/https#timeouts_and_retries
   // and: https://cloud.google.com/appengine/docs/standard/nodejs/how-instances-are-managed#timeout
@@ -219,6 +217,7 @@ const main = async (): Promise<void> => {
   listener.keepAliveTimeout = 630 * 1000 // 30s more than the 10min keepalive used by appengine.
   // And a workaround for node.js bug: https://github.com/nodejs/node/issues/27363
   listener.headersTimeout = 640 * 1000 // 10s more than above
+  listener.timeout = 640 * 1000 // match headersTimeout
 }
 
 // only call main if the file was called from the CLI and wasn't required from another module

packages/api/src/services/integrations/readwise.ts

@@ -125,6 +125,7 @@ export class ReadwiseIntegration extends IntegrationService {
             Authorization: `Token ${token}`,
             ContentType: 'application/json',
           },
+          timeout: 10000, // 10 seconds
         }
       )
       return response.status === 200

packages/api/src/services/save_email.ts

@@ -106,12 +106,8 @@ export const saveEmail = async (
 
   // create a task to update thumbnail and pre-cache all images
   try {
-    const taskId = await enqueueThumbnailTask(
-      ctx.uid,
-      slug,
-      articleToSave.content
-    )
-    logger.info('Created thumbnail task', taskId)
+    const taskId = await enqueueThumbnailTask(ctx.uid, slug)
+    logger.info('Created thumbnail task', { taskId })
   } catch (e) {
     logger.error('Failed to create thumbnail task', e)
   }

packages/api/src/services/save_page.ts

@@ -31,6 +31,7 @@ type SaveContext = {
   pubsub: PubsubClient
   models: DataModels
   uid: string
+  refresh?: boolean
 }
 
 type SaverUserData = {
@@ -186,12 +187,8 @@ export const savePage = async (
 
   // create a task to update thumbnail and pre-cache all images
   try {
-    const taskId = await enqueueThumbnailTask(
-      saver.userId,
-      slug,
-      articleToSave.content
-    )
-    logger.info('Created thumbnail task', taskId)
+    const taskId = await enqueueThumbnailTask(saver.userId, slug)
+    logger.info('Created thumbnail task', { taskId })
   } catch (e) {
     logger.error('Failed to create thumbnail task', e)
   }

packages/api/src/utils/createTask.ts

@@ -21,7 +21,7 @@ import View = google.cloud.tasks.v2.Task.View
 // Instantiates a client.
 const client = new CloudTasksClient()
 
-const logError = (error: Error): void => {
+const logError = (error: any): void => {
   if (axios.isAxiosError(error)) {
     logger.error(error.response)
   } else {
@@ -102,7 +102,12 @@ const createHttpTaskWithToken = async ({
       : null,
   }
 
-  return client.createTask({ parent, task })
+  try {
+    return client.createTask({ parent, task })
+  } catch (error) {
+    logError(error)
+    return null
+  }
 }
 
 export const createAppEngineTask = async ({
@@ -526,14 +531,12 @@ export const enqueueImportFromIntegration = async (
 
 export const enqueueThumbnailTask = async (
   userId: string,
-  slug: string,
-  content: string
+  slug: string
 ): Promise<string> => {
   const { GOOGLE_CLOUD_PROJECT } = process.env
   const payload = {
     userId,
     slug,
-    content,
   }
 
   const headers = {

packages/api/src/utils/helpers.ts

@@ -299,7 +299,7 @@ export const isUrl = (str: string): boolean => {
     validateUrl(str)
     return true
   } catch {
-    logger.error('not an url', str)
+    logger.info('not an url', { url: str })
     return false
   }
 }

packages/api/src/utils/imageproxy.ts

@@ -1,5 +1,5 @@
-import { encode } from 'urlsafe-base64'
 import crypto from 'crypto'
+import { encode } from 'urlsafe-base64'
 import { env } from '../env'
 
 function signImageProxyUrl(url: string): string {
@@ -17,6 +17,11 @@ export function createImageProxyUrl(
     return url
   }
 
+  // url is already signed
+  if (url.startsWith(env.imageProxy.url)) {
+    return url
+  }
+
   const urlWithOptions = `${url}#${width}x${height}`
   const signature = signImageProxyUrl(urlWithOptions)
 

packages/api/src/utils/logger.ts

@@ -97,18 +97,23 @@ function localConfig(id: string): ConsoleTransportOptions {
 const truncateObjectDeep = (object: any, length: number): any => {
   const copyObj = cloneDeep(object) as never
 
-  const truncateDeep = (obj: any): any => {
+  const truncateDeep = (obj: any, level: number): any => {
+    // reach maximum call stack size
+    if (level >= 5) {
+      return obj
+    }
+
     if (isString(obj) && obj.length > length) {
       return `${truncate(obj, { length })} [truncated]`
     }
 
     if (isArray(obj)) {
-      return obj.map((i) => truncateDeep(i) as never)
+      return obj.map((i) => truncateDeep(i, level + 1) as never)
     }
 
     if (isObject(obj)) {
       Object.entries(obj).forEach(([key, value]) => {
-        obj[key as keyof typeof obj] = truncateDeep(value) as never
+        obj[key as keyof typeof obj] = truncateDeep(value, level + 1) as never
       })
 
       return obj
@@ -118,7 +123,7 @@ const truncateObjectDeep = (object: any, length: number): any => {
     return obj
   }
 
-  return truncateDeep(copyObj)
+  return truncateDeep(copyObj, 1)
 }
 
 class GcpLoggingTransport extends LoggingWinston {

packages/api/src/utils/parser.ts

@@ -505,9 +505,9 @@ export const fetchFavicon = async (
     return `https://api.faviconkit.com/${domain}/128`
   } catch (e) {
     if (axios.isAxiosError(e)) {
-      logger.error('failed to get favicon:', e.response?.status)
+      logger.error('failed to get favicon', e.response)
     } else {
-      logger.error('failed to get favicon:', e)
+      logger.error('failed to get favicon', e)
     }
     return undefined
   }

packages/thumbnail-handler/package.json

@@ -27,6 +27,7 @@
     "dotenv": "^16.0.1",
     "image-size": "^1.0.2",
     "jsonwebtoken": "^8.5.1",
-    "linkedom": "^0.14.26"
+    "linkedom": "^0.14.26",
+    "urlsafe-base64": "^1.0.0"
   }
 }

packages/thumbnail-handler/src/index.ts

@@ -1,9 +1,11 @@
 import * as Sentry from '@sentry/serverless'
-import axios from 'axios'
+import axios, { AxiosResponse } from 'axios'
+import crypto from 'crypto'
 import * as dotenv from 'dotenv' // see https://github.com/motdotla/dotenv#how-do-i-use-dotenv-with-import
 import sizeOf from 'image-size'
 import * as jwt from 'jsonwebtoken'
 import { parseHTML } from 'linkedom'
+import { encode } from 'urlsafe-base64'
 import { promisify } from 'util'
 
 interface ArticleResponse {
@@ -30,7 +32,12 @@ interface UpdatePageResponse {
 
 interface ThumbnailRequest {
   slug: string
-  content: string
+}
+
+interface ImageSize {
+  src: string
+  width: number
+  height: number
 }
 
 dotenv.config()
@@ -42,6 +49,28 @@ Sentry.GCPFunction.init({
 const signToken = promisify(jwt.sign)
 const REQUEST_TIMEOUT = 30000 // 30s
 
+const signImageProxyUrl = (url: string, secret: string): string => {
+  return encode(crypto.createHmac('sha256', secret).update(url).digest())
+}
+
+export function createImageProxyUrl(
+  url: string,
+  width = 0,
+  height = 0
+): string {
+  if (!process.env.IMAGE_PROXY_URL || !process.env.IMAGE_PROXY_SECRET) {
+    return url
+  }
+
+  const urlWithOptions = `${url}#${width}x${height}`
+  const signature = signImageProxyUrl(
+    urlWithOptions,
+    process.env.IMAGE_PROXY_SECRET
+  )
+
+  return `${process.env.IMAGE_PROXY_URL}/${width}x${height},s${signature}/${url}`
+}
+
 const articleQuery = async (
   userId: string,
   slug: string
@@ -158,17 +187,30 @@ const updatePageMutation = async (
 }
 
 const isThumbnailRequest = (body: any): body is ThumbnailRequest => {
-  return 'slug' in body && 'content' in body
+  return 'slug' in body
 }
 
-const getImageSize = async (url: string): Promise<[number, number] | null> => {
+const fetchImage = async (url: string): Promise<AxiosResponse | null> => {
+  console.log('fetching image', url)
   try {
     // get image file by url
-    const response = await axios.get(url, {
+    return axios.get(url, {
       responseType: 'arraybuffer',
-      timeout: 5000, // 5s
-      maxContentLength: 10000000, // 10mb
+      timeout: 10000, // 10s
+      maxContentLength: 20000000, // 20mb
     })
+  } catch (e) {
+    console.log('fetch image error', e)
+    return null
+  }
+}
+
+const getImageSize = async (src: string): Promise<ImageSize | null> => {
+  try {
+    const response = await fetchImage(src)
+    if (!response) {
+      return null
+    }
 
     // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
     const buffer = Buffer.from(response.data, 'binary')
@@ -180,64 +222,75 @@ const getImageSize = async (url: string): Promise<[number, number] | null> => {
       return null
     }
 
-    return [width, height]
+    return {
+      src,
+      width,
+      height,
+    }
   } catch (e) {
     console.log(e)
     return null
   }
 }
 
-// credit: https://github.com/reddit-archive/reddit/blob/753b17407e9a9dca09558526805922de24133d53/r2/r2/lib/media.py#L706
-export const findThumbnail = async (
-  content: string
-): Promise<string | null> => {
+export const fetchAllImageSizes = async (content: string) => {
   const dom = parseHTML(content).document
 
-  // find the largest and squarest image as the thumbnail
-  // and pre-cache all images
+  // fetch all images by src and get their sizes
   const images = dom.querySelectorAll('img[src]')
   if (!images || images.length === 0) {
-    console.debug('no images')
-    return null
+    console.log('no images')
+    return []
   }
 
+  return Promise.all(
+    Array.from(images).map((image) => {
+      const src = image.getAttribute('src')
+      if (!src) {
+        return null
+      }
+
+      return getImageSize(src)
+    })
+  )
+}
+
+// credit: https://github.com/reddit-archive/reddit/blob/753b17407e9a9dca09558526805922de24133d53/r2/r2/lib/media.py#L706
+export const findThumbnail = (imagesSizes: (ImageSize | null)[]) => {
+  // find the largest and squarest image as the thumbnail
   let thumbnail = null
   let largestArea = 0
-  for await (const image of Array.from(images)) {
-    const src = image.getAttribute('src')
-    if (!src) {
+  for (const imageSize of Array.from(imagesSizes)) {
+    if (!imageSize) {
       continue
     }
 
-    const size = await getImageSize(src)
-    if (!size) {
-      continue
-    }
-
-    let area = size[0] * size[1]
+    let area = imageSize.width * imageSize.height
 
     // ignore small images
     if (area < 5000) {
-      console.debug('ignore small', src)
+      console.log('ignore small', imageSize.src)
       continue
     }
 
     // penalize excessively long/wide images
-    const ratio = Math.max(...size) / Math.min(...size)
+    const ratio =
+      Math.max(imageSize.width, imageSize.height) /
+      Math.min(imageSize.width, imageSize.height)
     if (ratio > 1.5) {
-      console.debug('penalizing long/wide', src)
+      console.log('penalizing long/wide', imageSize.src)
       area /= ratio * 2
     }
 
     // penalize images with "sprite" in their name
-    if (src.toLowerCase().includes('sprite')) {
-      console.debug('penalizing sprite', src)
+    if (imageSize.src.toLowerCase().includes('sprite')) {
+      console.log('penalizing sprite', imageSize.src)
       area /= 10
     }
 
     if (area > largestArea) {
       largestArea = area
-      thumbnail = src
+      thumbnail = imageSize.src
     }
   }
 
@@ -280,30 +333,42 @@ export const thumbnailHandler = Sentry.GCPFunction.wrapHttpFunction(
       return res.status(400).send('BAD_REQUEST')
     }
 
-    const { slug, content } = req.body
+    const { slug } = req.body
 
     try {
-      // find thumbnail from all images & pre-cache
-      const thumbnail = await findThumbnail(content)
-      if (!thumbnail) {
-        console.debug('no thumbnail')
-        return res.status(200).send('NOT_FOUND')
-      }
-
       const page = await articleQuery(uid, slug)
       if (!page) {
         console.info('page not found')
         return res.status(200).send('NOT_FOUND')
       }
 
-      // update page with thumbnail if not already set
       if (page.image) {
-        console.debug('thumbnail already set')
-        return res.status(200).send('OK')
+        console.log('thumbnail already set')
+        // pre-cache thumbnail first if exists
+        const imageProxyUrl = createImageProxyUrl(page.image, 320, 320)
+        const image = await fetchImage(imageProxyUrl)
+        if (!image) {
+          console.log('thumbnail image not found')
+          page.image = undefined
+        }
       }
 
-      const updated = await updatePageMutation(uid, page.id, thumbnail)
-      console.debug('thumbnail updated', updated)
+      console.log('pre-caching all images...')
+      // pre-cache all images in the content and get their sizes
+      const imageSizes = await fetchAllImageSizes(page.content)
+      // find thumbnail from all images if thumbnail not set
+      if (!page.image && imageSizes.length > 0) {
+        console.log('finding thumbnail...')
+        const thumbnail = findThumbnail(imageSizes)
+        if (!thumbnail) {
+          console.log('no thumbnail found from content')
+          return res.status(200).send('NOT_FOUND')
+        }
+
+        // update page with thumbnail
+        const updated = await updatePageMutation(uid, page.id, thumbnail)
+        console.log('thumbnail updated', updated)
+      }
 
       res.send('ok')
     } catch (e) {

packages/thumbnail-handler/test/index.test.ts

@@ -3,7 +3,7 @@ import fs from 'fs'
 import 'mocha'
 import nock from 'nock'
 import path from 'path'
-import { findThumbnail } from '../src'
+import { fetchAllImageSizes, findThumbnail } from '../src'
 
 describe('findThumbnail', () => {
   it('finds the largest and squarest image', async () => {
@@ -20,7 +20,8 @@ describe('findThumbnail', () => {
       'utf8'
     )
     // find thumbnail
-    const thumbnail = await findThumbnail(content)
+    const imageSizes = await fetchAllImageSizes(content)
+    const thumbnail = findThumbnail(imageSizes)
 
     expect(thumbnail).to.eql('https://omnivore.app/large_and_square.png')
   })