Merge pull request #85 from highcharts/enhancement/dedupe-gh-requests

Enhancement/dedupe gh requests

Author: goransle

.github/copilot-instructions.md

@@ -0,0 +1,24 @@
+# Copilot PR Review Instructions
+
+## Review scope
+- Focus on correctness, security, performance, and deprecated APIs.
+- Flag missing tests when behaviour changes or new features land.
+- Prefer precise, actionable comments over summaries when issues exist.
+- For stylistic nits, suggest fixes only if the repository enforces them.
+
+## Required checks before approval
+- Ensure the description explains *what* changed and *why*.
+- Verify that new endpoints, configs, or scripts are documented.
+- Confirm migrations or data changes include rollback notes.
+- Look for added secrets, credentials, or tokens committed by mistake.
+
+## Tests and validation
+- Confirm automated tests cover the new functionality.
+- If tests are absent or incomplete, request them or supply a test plan summarising the manual steps required to validate the change.
+- For flaky or long-running suites, suggest targeted smoke tests that cover the risk surface.
+
+## Tone and style
+- Keep feedback concise, professional, and specific.
+- Reference files and lines when calling out problems.
+- Offer concrete remediation ideas alongside critiques.
+- Acknowledge improvements or clever solutions when applicable.

README.md

@@ -18,7 +18,7 @@ The application requires a configuration file `./config.json` to be able to run.
 
 | Setting | Description |
 |---|---|
-| informationLevel | The level of severity of the information outputted to the log. The severity can be 0 (everything), 1 (warnings and errors), 2 (only errors). Defaults to 2. |
+| informationLevel | The level of severity of the information outputted to the log. The severity can be 0 (everything), 1 (warnings and errors), 2 (only errors). Defaults to 2. You can override this per runtime by setting the `INFORMATION_LEVEL` environment variable. |
 | port | The port the server application will listen to. Defaults to 80. |
 | secureToken | The secret token used to validate the GitHub webhook post to /update. See [GitHub Developer - Securing your webhooks](https://developer.github.com/webhooks/securing/) for more information. |
 | token | Personal access token used to gain access to GitHub API. The token scope is requires only access to public repositories. See [GitHub Help - Creating a personal access token for the command line](https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line) for more information. |
@@ -38,6 +38,14 @@ Example config file:
 }
 ```
 
+To override logging without editing `config.json`, set an environment variable before starting the server, for example:
+
+```bash
+INFORMATION_LEVEL=0 npm start
+```
+
+Environment variables take precedence over values in `config.json`.
+
 ## Run the application
 Open a CLI and run the command: `npm start`
 

app/JobQueue.js

@@ -24,14 +24,14 @@ class JobQueue {
   async doJob (queue, jobID) {
     const job = queue.get(jobID)
     if (job) {
-      job.func(...job.args)
-        .catch((error) => {
-          console.log(error)
-        })
-        .finally(() => {
-          queue.delete(jobID)
-          job.setDone(true)
-        })
+      try {
+        await Promise.resolve().then(() => job.func(...job.args))
+      } catch (error) {
+        console.log(error)
+      } finally {
+        queue.delete(jobID)
+        job.setDone(true)
+      }
       await job.done
     }
   }

app/JobQueue.js.map

@@ -4,7 +4,7 @@ const { join } = require('node:path')
 const { stat, opendir, readFile, writeFile, symlink } = require('node:fs/promises')
 const { existsSync } = require('node:fs')
 const { downloadSourceFolder, getBranchInfo, getCommitInfo } = require('./download')
-const { compileTypeScript } = require('./utilities')
+const { compileTypeScript, log } = require('./utilities')
 const { build } = require('@highcharts/highcharts-assembler/src/build')
 const { JobQueue } = require('./JobQueue')
 
@@ -51,15 +51,18 @@ async function assembleDashboards (pathCacheDirectory, commit) {
       namespace: 'Dashboards'
     })
   } catch (error) {
-    console.log('assembler error: ', error)
+    log(2, `Dashboards assembler error: ${error.message}`)
   }
 
   /**
       * @param {string} dirPath
       */
   async function modifyFiles (dirPath) {
     const dir = await opendir(dirPath)
-      .catch(() => null)
+      .catch(error => {
+        log(2, `Failed to open directory ${dirPath}: ${error.message}`)
+        return null
+      })
 
     if (dir) {
       for await (const dirent of dir) {
@@ -78,7 +81,10 @@ async function assembleDashboards (pathCacheDirectory, commit) {
           await symlink(
             join(dirPath, dirent.name),
             join(dirPath, dirent.name.replace('.src', ''))
-          ).catch(() => null)
+          ).catch(error => {
+            log(2, `Failed to create symlink in ${dirPath}: ${error.message}`)
+            return null
+          })
         }
       }
     }
@@ -142,7 +148,7 @@ async function dashboardsHandler (req, res, next) {
     await stat(join(pathCacheDirectory, 'js'))
   } catch (err) {
     if (err.code === 'ENOENT') {
-      await queue.addJob(
+      await Promise.resolve().then(() => queue.addJob(
         'download',
         commit,
         {
@@ -153,7 +159,10 @@ async function dashboardsHandler (req, res, next) {
             commit
           ]
         }
-      ).catch(handleQueueError)
+      )).catch(error => {
+        log(2, `Failed to enqueue download for ${commit}: ${error.message}`)
+        return handleQueueError(error)
+      })
     } else {
       return res.sendStatus(404)
     }
@@ -188,7 +197,7 @@ async function dashboardsHandler (req, res, next) {
       : Promise.resolve(false),
     // try to build and assemble file
     async () => {
-      await queue.addJob(
+      await Promise.resolve().then(() => queue.addJob(
         'compile',
         commit + filepath,
         {
@@ -199,16 +208,23 @@ async function dashboardsHandler (req, res, next) {
               ? obj.compile
               : obj.compile.replace('.js', '.src.js')
           ]
-        }).catch(handleQueueError)
+        }
+      )).catch(error => {
+        log(2, `Failed to enqueue TypeScript compile for ${commit}${filepath}: ${error.message}`)
+        return handleQueueError(error)
+      })
 
-      await queue.addJob(
+      await Promise.resolve().then(() => queue.addJob(
         'compile',
         commit + filepath,
         {
           func: assembleDashboards,
           args: [pathCacheDirectory, commit]
         }
-      ).catch(handleQueueError)
+      )).catch(error => {
+        log(2, `Failed to enqueue dashboard assembly for ${commit}${filepath}: ${error.message}`)
+        return handleQueueError(error)
+      })
 
       res.status(201)
 

app/dashboards.js

@@ -15,6 +15,51 @@ const authToken = { Authorization: `token ${token}` }
 
 const degit = require('tiged')
 
+const DEFAULT_CACHE_TTL = Number(process.env.GITHUB_LOOKUP_CACHE_TTL || 60_000)
+const NEGATIVE_CACHE_TTL = Number(process.env.GITHUB_LOOKUP_NEGATIVE_CACHE_TTL || 10_000)
+
+const branchInfoCache = new Map()
+const commitInfoCache = new Map()
+let githubRequest
+
+/**
+ * Resolve a value from cache or execute the provided fetcher.
+ * Shares in-flight fetches and caches both positive and negative responses.
+ * @template T
+ * @param {Map<string, { expires?: number, value?: T, promise?: Promise<T> }>} cache
+ * @param {string} key
+ * @param {() => Promise<T>} fetcher
+ * @returns {Promise<T>}
+ */
+function getWithCache (cache, key, fetcher) {
+  const now = Date.now()
+  const cached = cache.get(key)
+
+  if (cached) {
+    if (cached.expires && cached.expires > now && ('value' in cached)) {
+      return Promise.resolve(cached.value)
+    }
+    if (cached.promise) {
+      return cached.promise
+    }
+  }
+
+  const promise = fetcher().then(result => {
+    const ttl = result ? DEFAULT_CACHE_TTL : NEGATIVE_CACHE_TTL
+    cache.set(key, {
+      value: result,
+      expires: Date.now() + ttl
+    })
+    return result
+  }).catch(error => {
+    cache.delete(key)
+    throw error
+  })
+
+  cache.set(key, { promise })
+  return promise
+}
+
 /**
  * Downloads the content of a url and writes it to the given output path.
  * The Promise is resolved with an object containing information of the result
@@ -33,7 +78,7 @@ async function downloadFile (url, outputPath) {
   }
 
   if (statusCode === 200) {
-    console.log(`Downloading ${url}`)
+    log(0, `Downloading ${url}`)
     await writeFile(outputPath, body)
     result.success = true
   }
@@ -180,6 +225,8 @@ function get(options) {
     })
 }
 
+githubRequest = get
+
 /**
  * Gives a list of all the source files in the given branch in the repository.
  * The Promise resolves with a list of objects containing information on each
@@ -273,19 +320,21 @@ async function urlExists(url) {
  * @returns {Promise<({}|false)>}
  * The branch info object, or false if not found
  */
-async function getBranchInfo(branch) {
-    const { body, statusCode } = await get({
-        hostname: 'api.github.com',
-        path: `/repos/${repo}/branches/${branch}`,
-        headers: {
-            'user-agent': 'github.highcharts.com',
-            ...authToken
-        }
+async function getBranchInfo (branch) {
+  return getWithCache(branchInfoCache, branch, async () => {
+    const { body, statusCode } = await githubRequest({
+      hostname: 'api.github.com',
+      path: `/repos/${repo}/branches/${branch}`,
+      headers: {
+        'user-agent': 'github.highcharts.com',
+        ...authToken
+      }
     })
     if (statusCode === 200) {
-        return JSON.parse(body)
+      return JSON.parse(body)
     }
     return false
+  })
 }
 
 
@@ -297,19 +346,30 @@ async function getBranchInfo(branch) {
  * @returns {Promise<({}|false)>}
  * The commit info object, or false if not found
  */
-async function getCommitInfo(commit) {
-    const { body, statusCode } = await get({
-        hostname: 'api.github.com',
-        path: `/repos/${repo}/commits/${commit}`,
-        headers: {
-            'user-agent': 'github.highcharts.com',
-            ...authToken
-        }
+async function getCommitInfo (commit) {
+  return getWithCache(commitInfoCache, commit, async () => {
+    const { body, statusCode } = await githubRequest({
+      hostname: 'api.github.com',
+      path: `/repos/${repo}/commits/${commit}`,
+      headers: {
+        'user-agent': 'github.highcharts.com',
+        ...authToken
+      }
     })
     if (statusCode === 200) {
-        return JSON.parse(body)
+      return JSON.parse(body)
     }
     return false
+  })
+}
+
+function setGitHubRequest (fn) {
+  githubRequest = typeof fn === 'function' ? fn : get
+}
+
+function clearGitHubCache () {
+  branchInfoCache.clear()
+  commitInfoCache.clear()
 }
 
 // Export download functions
@@ -322,5 +382,7 @@ module.exports = {
     httpsGetPromise: get,
     urlExists,
     getBranchInfo,
-    getCommitInfo
+    getCommitInfo,
+    __setGitHubRequest: setGitHubRequest,
+    __clearGitHubCache: clearGitHubCache
 }

app/download.js

@@ -57,6 +57,7 @@ async function fsStat (path) {
   try {
     return await stat(path)
   } catch (e) {
+    log(2, `fsStat failed for ${path}: ${e.message}`)
     return false
   }
 }

app/filesystem.js

@@ -55,6 +55,9 @@ const queue = new JobQueue()
 async function shouldDownloadTypeScriptFolders (repoURL, branch) {
   const urlPath = `${repoURL}${branch}/ts/tsconfig.json`
   const tsConfigPath = join(PATH_TMP_DIRECTORY, branch, 'ts', 'tsconfig.json')
+  if (exists(tsConfigPath)) {
+    return true
+  }
   const tsConfigResponse = await downloadFile(urlPath, tsConfigPath)
 
   return (tsConfigResponse.statusCode >= 200 && tsConfigResponse.statusCode < 300)
@@ -302,6 +305,7 @@ async function serveBuildFile (branch, requestURL, useGitDownloader = true) {
         return { status: 202, body: error.message }
       }
 
+      log(2, `Queue addJob failed for ${branch}: ${error.message}`)
       return { status: 200 }
     })
 

app/handlers.js

@@ -22,12 +22,15 @@ const util = require('util')
 console.log(tscPath)
 
 // Import dependencies, sorted by path.
-const config = require('../config.json')
-
 // Constants
-const INFORMATION_LEVEL = typeof config.informationLevel === 'number'
-  ? config.informationLevel
-  : 2
+const envInformationLevel = process.env.INFORMATION_LEVEL
+const INFORMATION_LEVEL = (() => {
+  if (typeof envInformationLevel !== 'undefined') {
+    const parsed = Number(envInformationLevel)
+    return Number.isFinite(parsed) ? parsed : 2
+  }
+  return 2
+})()
 
 /**
  * Format a date as YYYY-MM-DDTHH-MM-SS. Returns a string with date formatted