Merge commit '9ba44827cabe1eb319f03e24fad27fe709e25834' into 682-data…

…set-task-list-fetch-data-from-all-active-resources

config/default.yaml

@@ -42,6 +42,10 @@ email: {
 validations: {
     maxFileSize: 100000000
 }
+contact:
+  issues:
+    # description: should be used for general issues, e.g. if user lands on 404 doesn't know how to proceed
+    email: [email protected]
 datasetsConfig:
   article-4-direction:
     guidanceUrl: /guidance/specifications/article-4-direction

config/util.js

@@ -94,7 +94,12 @@ export const ConfigSchema = v.object({
       measurementId: v.string()
     })
   ),
-  tablePageLength: v.number()
+  tablePageLength: v.number(),
+  contact: v.object({
+    issues: v.object({
+      email: v.pipe(v.string(), v.email())
+    })
+  })
 })
 
 const readConfig = (config) => {

package.json

@@ -6,15 +6,15 @@
   "type": "module",
   "scripts": {
     "prepare": "husky",
-    "dev": "docker-compose -f docker-compose-real-backend-minus-frontend.yml up -d && npm run start:local",
+    "dev": "docker compose -f docker-compose-real-backend-minus-frontend.yml up -d && npm run start:local",
     "start": "node index.js",
     "start:watch": "concurrently \"nodemon --exec 'npm run build && npm run start'\" \"npm run scss:watch\"",
     "start:test": "NODE_ENV=test node index.js",
     "start:local": "NODE_ENV=local node index.js",
-    "start:wiremock": "docker-compose up -d --force-recreate --build && NODE_ENV=wiremock node index.js",
+    "start:wiremock": "docker compose up -d --force-recreate --build && NODE_ENV=wiremock node index.js",
     "start:development": "NODE_ENV=development node index.js",
     "start:local:watch": "NODE_ENV=test npm run start:watch",
-    "docker-security-scan": "docker-compose -f docker-compose.security.yml run --rm zap",
+    "docker-security-scan": "docker compose -f docker-compose.security.yml run --rm zap",
     "static-security-scan": "npm audit --json > npm-audit-report.json",
     "scan:zap": "npm run docker-security-scan && npm run static-security-scan",
     "mock:api": "node ./test/mock-api/index.js",

readme.md

@@ -102,11 +102,11 @@ of [package.json](package.json)) for more examples.
     ```
 - Run the application using docker
     ```
-    docker-compose -f docker-compose-real-backend.yml up
+    docker compose -f docker-compose-real-backend.yml up
     ```
 - Run the application (without the frontend) using docker
     ```
-    docker-compose -f docker-compose-real-backend-minus-frontend.yml up
+    docker compose -f docker-compose-real-backend-minus-frontend.yml up
     ```
 - Run external services in containers and start application
     ```

src/assets/scss/index.scss

@@ -215,3 +215,11 @@ code * {
 .app-content__markdown img {
   max-width: 100%;
 }
+
+.schema-issue {
+  display: flex;
+  flex-direction: column;
+  padding-bottom: 1em;
+}
+
+.schema-issue p { padding: 0; margin: 0}

src/filters/filters.js

@@ -8,6 +8,7 @@ import { checkToolDeepLink } from './checkToolDeepLink.js'
 import pluralize from 'pluralize'
 import { datasetSlugToReadableName } from '../utils/datasetSlugToReadableName.js'
 import { getDatasetGuidanceUrl } from './getDatasetConfig.js'
+import { schemaIssues } from './schemaIssues.js'
 
 /** maps dataset status (as returned by `fetchLpaOverview` middleware to a
  * CSS class used by the govuk-tag component
@@ -43,6 +44,7 @@ const addFilters = (nunjucksEnv) => {
   nunjucksEnv.addFilter('pluralise', pluralize)
   nunjucksEnv.addFilter('checkToolDeepLink', checkToolDeepLink)
   nunjucksEnv.addFilter('getDatasetGuidanceUrl', getDatasetGuidanceUrl)
+  nunjucksEnv.addFilter('schemaIssues', schemaIssues)
 }
 
 export default addFilters

src/filters/schemaIssues.js

@@ -0,0 +1,21 @@
+import * as v from 'valibot'
+
+/**
+ * Takes an Error and if it's a ValiError, returns an array of issue summaries (with paths). Empty array otherwise.
+ *
+ * Useful to show the schema issues in a consise way.
+ *
+ * @param {Error} error
+ * @returns { { path: string[], message: string}[] }
+ */
+export function schemaIssues (error) {
+  const issues = []
+  if (v.isValiError(error)) {
+    for (const issue of error.issues) {
+      if (issue.path) {
+        issues.push({ path: issue.path.map(elem => elem.key), message: issue.message })
+      }
+    }
+  }
+  return issues
+}

src/middleware/common.middleware.js

@@ -7,6 +7,7 @@ import { fetchMany, fetchOne, FetchOneFallbackPolicy, FetchOptions, renderTempla
 import * as v from 'valibot'
 import { pagination } from '../utils/pagination.js'
 import datasette from '../services/datasette.js'
+import { errorTemplateContext, MiddlewareError } from '../utils/errors.js'
 
 /**
  * Middleware. Set `req.handlerName` to a string that will identify
@@ -98,7 +99,8 @@ export function validateQueryParamsFn (req, res, next) {
     req.parsedParams = v.parse(this.schema || v.any(), req.params)
     next()
   } catch (error) {
-    res.status(400).render('errorPages/400', {})
+    const err = new MiddlewareError('Query params validation error', 400, { cause: error })
+    res.status(err.statusCode).render(err.template, { ...errorTemplateContext(), err })
   }
 }
 
@@ -127,9 +129,7 @@ export const show404IfPageNumberNotInRange = (req, res, next) => {
   }
 
   if (pageNumber > dataRange.maxPageNumber || pageNumber < 1) {
-    const error = new Error('page number not in range')
-    // @ts-ignore
-    error.status = 404
+    const error = new MiddlewareError('page number not in range', 404)
     return next(error)
   }
   next()
@@ -676,7 +676,7 @@ export function getErrorSummaryItems (req, res, next) {
 
 export const prepareIssueDetailsTemplateParams = (req, res, next) => {
   const { entry, pagination, dataRange, errorSummary, dataset, orgInfo } = req
-  const { issue_type: issueType, pageNumber } = req.parsedParams
+  const { issue_type: issueType, issue_field: issueField, pageNumber } = req.parsedParams
 
   // schema: OrgIssueDetails
   req.templateParams = {
@@ -685,6 +685,7 @@ export const prepareIssueDetailsTemplateParams = (req, res, next) => {
     errorSummary,
     entry,
     issueType,
+    issueField,
     pagination,
     pageNumber,
     dataRange
@@ -706,7 +707,7 @@ export const fetchSources = fetchMany({
       SELECT
         rhe.endpoint,
         rhe.endpoint_url,
-        case 
+        case
             when rhe.status = '' or rhe.status is null then null
             else cast(rhe.status as int)
         end as status,

src/middleware/entryIssueDetails.middleware.js

@@ -1,5 +1,6 @@
 import * as v from 'valibot'
 import { createPaginationTemplateParams, fetchDatasetInfo, fetchEntryIssues, fetchOrgInfo, fetchResources, getErrorSummaryItems, getSetBaseSubPath, getSetDataRange, prepareIssueDetailsTemplateParams, show404IfPageNumberNotInRange, validateQueryParams } from './common.middleware.js'
+import { MiddlewareError } from '../utils/errors.js'
 import { fetchMany, fetchOne, FetchOptions, renderTemplate } from './middleware.builders.js'
 import { issueErrorMessageHtml } from '../utils/utils.js'
 
@@ -62,8 +63,12 @@ export const prepareEntry = (req, res, next) => {
   const { resources, entryIssues } = req
 
   if (!entryIssues || entryIssues.length === 0 || !resources || resources.length === 0) {
-    const error = new Error('Missing required values on request object')
-    error.status = 404
+    const details = [
+      `entryIssues: ${entryIssues ? 'present' : 'missing'}`,
+      `entryIssues[0]: ${entryIssues[0] ? 'present' : 'missing'}`,
+      `resources: ${resources ? 'present' : 'missing'}`
+    ].join(', ')
+    const error = new MiddlewareError(`Missing required values on request object: ${details}`, 404)
     return next(error)
   }
 

src/middleware/middleware.builders.js

@@ -13,6 +13,7 @@ import { templateSchema } from '../routes/schemas.js'
 import { render } from '../utils/custom-renderer.js'
 import datasette from '../services/datasette.js'
 import * as v from 'valibot'
+import { errorTemplateContext, MiddlewareError } from '../utils/errors.js'
 
 export const FetchOptions = {
   /**
@@ -30,7 +31,8 @@ const datasetOverride = (val, req) => {
     return 'digital-land'
   }
   if (val === FetchOptions.fromParams) {
-    console.assert(req.params.dataset, 'no "dataset" in request params')
+    logger.warn('no "dataset" in request params',
+      { types: types.App, endpoint: req.originalUrl, params: req.params })
     return req.params.dataset
   } else if (val === FetchOptions.performanceDb) {
     return 'performance'
@@ -40,7 +42,8 @@ const datasetOverride = (val, req) => {
 }
 
 const fetchOneFallbackPolicy = (req, res, next) => {
-  res.status(404).render('errorPages/404', {})
+  const err = new MiddlewareError('Not found', 404)
+  res.status(err.statusCode).render(err.template, { ...errorTemplateContext(), err })
 }
 
 /**

src/routes/guidance.js

@@ -1,7 +1,4 @@
 import express from 'express'
-/* import nunjucks from 'nunjucks'
-import config from '../../config/index.js'
-import logger from '../utils/logger.js' */
 
 const router = express.Router()
 
@@ -11,82 +8,4 @@ router.get('/*', (req, res) => {
   return res.redirect(302, `https://www.planning.data.gov.uk/guidance/${path}`)
 })
 
-/* function getNavigationStructure () {
-  if (!config.guidanceNavigation) {
-    logger.error('Guidance navigation configuration is missing')
-    return { title: 'Guidance', items: [] }
-  }
-
-  return config.guidanceNavigation
-} */
-
-/* function extractUrlsFromItems (items) {
-  let urls = []
-  items.forEach(item => {
-    if (item.url) {
-      urls.push(item.url)
-    }
-    if (item.items) {
-      urls = urls.concat(extractUrlsFromItems(item.items))
-    }
-  })
-  return urls
-}
-
-function checkPathExists (items, path) {
-  const fullPath = `/guidance${path}`.replace(/\/$/, '')
-  const urls = extractUrlsFromItems(items)
-
-  return urls.includes(fullPath)
-}
-
-router.get('/*', (req, res) => {
-  const path = req.params[0].replace(/[^a-zA-Z0-9/-]/g, '')
-  return res.redirect(302, `https://www.planning.data.gov.uk/guidance${path}`)
-
-  try {
-    const navigationStructure = getNavigationStructure()
-    const path = req.params[0].replace(/[^a-zA-Z0-9/-]/g, '')
-
-    if (path.includes('..') || !checkPathExists(navigationStructure.items, req.path)) {
-      throw new Error('Invalid path')
-    }
-
-    let templatePath = ''
-
-    switch (path) {
-      case '':
-      case '/':
-        templatePath = 'guidance/index'
-        break
-      case 'specifications':
-      case 'specifications/':
-        templatePath = 'guidance/specifications/index'
-        break
-      default:
-        templatePath = `guidance/${path}`
-    }
-
-    const guidancePage = nunjucks.render(`${templatePath}.md`, {
-      navigation: navigationStructure
-    })
-
-    res.send(guidancePage)
-  } catch (error) {
-    if (error?.message === 'template not found' || error?.message === 'Invalid path') {
-      logger.info('Guidance page not found', { type: 'App', path: req.path })
-
-      res.status(404).render('errorPages/404', {})
-    } else {
-      logger.error('Error rendering guidance page', {
-        type: 'App',
-        path: req.path,
-        error: error.message
-      })
-
-      res.status(500).render('errorPages/500', {})
-    }
-  }
-}) */
-
 export default router

src/routes/schemas.js

@@ -4,14 +4,16 @@
  */
 
 import * as v from 'valibot'
+import { MiddlewareError } from '../utils/errors.js'
 
 export const EmptyParams = v.object({})
-export const UptimeParams = v.object({
-  upTime: v.string()
-})
 
-export const ErrorParams = v.strictObject({
-  err: v.object({})
+export const ErrorPageParams = v.object({
+  err: v.instance(MiddlewareError),
+  env: v.string(),
+  supportEmail: v.pipe(v.string(), v.email()),
+  uptime: v.optional(v.string()),
+  downtime: v.optional(v.string())
 })
 
 export const NonEmptyString = v.pipe(v.string(), v.nonEmpty())
@@ -220,6 +222,7 @@ export const OrgIssueDetails = v.strictObject({
   dataset: DatasetNameField,
   errorSummary: errorSummaryParams,
   issueType: NonEmptyString,
+  issueField: NonEmptyString,
   entry: v.strictObject({
     title: NonEmptyString,
     fields: v.array(v.strictObject({
@@ -301,9 +304,7 @@ export const templateSchema = new Map([
   ['organisations/issueTable.html', OrgIssueTable],
   ['organisations/issueDetails.html', OrgIssueDetails],
 
-  ['errorPages/503', UptimeParams],
-  ['errorPages/500', ErrorParams],
-  ['errorPages/404', EmptyParams],
+  ['errorPages/error.njk', ErrorPageParams],
   ['privacy-notice.html', EmptyParams],
   ['landing.html', EmptyParams],
   ['cookies.html', EmptyParams],

src/serverSetup/errorHandlers.js

@@ -1,7 +1,9 @@
 import logger from '../utils/logger.js'
 import { types } from '../utils/logging.js'
+import { MiddlewareError, errorTemplateContext } from '../utils/errors.js'
 
 export function setupErrorHandlers (app) {
+  const errContext = errorTemplateContext()
   app.use((err, req, res, next) => {
     logger.error({
       type: types.Response,
@@ -17,17 +19,19 @@ export function setupErrorHandlers (app) {
       return next(err)
     }
 
-    err.template = err.template || (err.status && `errorPages/${err.status}`) || 'errorPages/500'
-
     if (err.redirect) {
       return res.redirect(err.redirect)
     }
 
-    err.status = err.status || 500
+    const middlewareError = err instanceof MiddlewareError ? err : new MiddlewareError('Internal server error', 500, { cause: err })
     try {
-      res.status(err.status).render(err.template, { err })
+      res.status(middlewareError.statusCode).render(middlewareError.template, { ...errContext, err: middlewareError })
     } catch (e) {
-      res.status(err.status).render('errorPages/500', { err })
+      logger.error('Failed to render error page.', {
+        type: types.Response, errorMessage: e.message, errorStack: e.stack, originalError: err.message, endpoint: req.originalUrl
+      })
+      const renderError = new MiddlewareError('Failed to render error page', 500, { cause: e })
+      res.status(500).render('errorPages/error.njk', { ...errContext, err: renderError })
     }
   })
 
@@ -41,6 +45,7 @@ export function setupErrorHandlers (app) {
       endpoint: req.originalUrl,
       message: 'not found'
     })
-    res.status(404).render('errorPages/404')
+    const err = new MiddlewareError('Not found', 404)
+    res.status(err.statusCode).render(err.template, { ...errContext, err })
   })
 }