diff --git a/config/default.yaml b/config/default.yaml index 2224d7b98..61dcb21ea 100644 --- a/config/default.yaml +++ b/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: digitalland@communities.gov.uk datasetsConfig: article-4-direction: guidanceUrl: /guidance/specifications/article-4-direction diff --git a/config/util.js b/config/util.js index a1463f664..2f004de7b 100644 --- a/config/util.js +++ b/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) => { diff --git a/package.json b/package.json index 5d5015bd2..091d5a05a 100644 --- a/package.json +++ b/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", diff --git a/readme.md b/readme.md index 1ff605f96..f07d48249 100644 --- a/readme.md +++ b/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 ``` diff --git a/src/assets/scss/index.scss b/src/assets/scss/index.scss index bc4e20020..95ffe2a2c 100644 --- a/src/assets/scss/index.scss +++ b/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} diff --git a/src/filters/filters.js b/src/filters/filters.js index 9bd633ef9..02bd16f5a 100644 --- a/src/filters/filters.js +++ b/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 diff --git a/src/filters/schemaIssues.js b/src/filters/schemaIssues.js new file mode 100644 index 000000000..31d923d48 --- /dev/null +++ b/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 +} diff --git a/src/middleware/common.middleware.js b/src/middleware/common.middleware.js index e3ce13cdb..12364179b 100644 --- a/src/middleware/common.middleware.js +++ b/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, diff --git a/src/middleware/entryIssueDetails.middleware.js b/src/middleware/entryIssueDetails.middleware.js index cf632cf9b..cf6c4f886 100644 --- a/src/middleware/entryIssueDetails.middleware.js +++ b/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) } diff --git a/src/middleware/middleware.builders.js b/src/middleware/middleware.builders.js index 518fea9be..fe0d7639b 100644 --- a/src/middleware/middleware.builders.js +++ b/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 }) } /** diff --git a/src/routes/guidance.js b/src/routes/guidance.js index 5a3e79157..b0f9edeb2 100644 --- a/src/routes/guidance.js +++ b/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 diff --git a/src/routes/schemas.js b/src/routes/schemas.js index 5a5117bfb..279fd6f42 100644 --- a/src/routes/schemas.js +++ b/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], diff --git a/src/serverSetup/errorHandlers.js b/src/serverSetup/errorHandlers.js index a8a6f692e..e9fc00a5d 100644 --- a/src/serverSetup/errorHandlers.js +++ b/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 }) }) } diff --git a/src/serverSetup/middlewares.js b/src/serverSetup/middlewares.js index bf2220b67..0ad424640 100644 --- a/src/serverSetup/middlewares.js +++ b/src/serverSetup/middlewares.js @@ -6,6 +6,7 @@ import { types } from '../utils/logging.js' import hash from '../utils/hasher.js' import config from '../../config/index.js' import { preventIndexing } from '../middleware/common.middleware.js' +import { MiddlewareError, errorTemplateContext } from '../utils/errors.js' export function setupMiddlewares (app) { app.use((req, res, next) => { @@ -34,7 +35,8 @@ export function setupMiddlewares (app) { app.use((req, res, next) => { const serviceDown = config.maintenance.serviceUnavailable || false if (serviceDown) { - res.status(503).render('errorPages/503', { upTime: config.maintenance.upTime }) + const err = new MiddlewareError('Service unavailable', 503) + res.status(err.statusCode).render(err.template, { ...errorTemplateContext(), err, uptime: config.maintenance.upTime }) } else { next() } diff --git a/src/services/asyncRequestApi.js b/src/services/asyncRequestApi.js index e96f7ea59..5e4d938de 100644 --- a/src/services/asyncRequestApi.js +++ b/src/services/asyncRequestApi.js @@ -60,6 +60,6 @@ export const getRequestData = async (resultId) => { return new ResultData(response.data) } catch (error) { - throw new Error(`HTTP error! status: ${error.status}`) + throw new Error(`HTTP error! status: ${error.status}: ${error.message}`, { cause: error }) } } diff --git a/src/services/datasette.js b/src/services/datasette.js index f90a9639a..f5366c74b 100644 --- a/src/services/datasette.js +++ b/src/services/datasette.js @@ -9,6 +9,7 @@ export default { * Executes a SQL query on the Datasette instance and returns the results. * * @param {string} query - The SQL query to execute. + * @param {string} database - The name of the database to query. Defaults to 'digital-land'. * @returns {Promise<{data: object, formattedData: object[]}>} - A promise that resolves to an object with the following properties: * - `data`: The raw data returned by Datasette. * - `formattedData`: The formatted data, with columns and rows parsed into a usable format. diff --git a/src/utils/errors.js b/src/utils/errors.js new file mode 100644 index 000000000..13d1ff84c --- /dev/null +++ b/src/utils/errors.js @@ -0,0 +1,31 @@ +import config from '../../config/index.js' + +const context = { + env: process.env.NODE_ENV || process.env.ENVIRONMENT || 'local', + supportEmail: config.contact.issues.email +} + +export function errorTemplateContext () { + return { ...context } +} + +/** + * Use this class if you want to display specific HTTP error page. + * + * Uses the `errorPages/error.njk` template, but it can be overriden via options. + */ +export class MiddlewareError extends Error { + /** + * @param {string} message + * @param {number} statusCode status to be returned to the client + * @param {{template?: string, cause?: Error}} options template path + */ + constructor (message, statusCode, options = {}) { + super(message, options) + if (typeof statusCode !== 'number') { + throw new Error(`statusCode should be a number: ${statusCode}`) + } + this.statusCode = statusCode + this.template = options?.template ?? 'errorPages/error.njk' + } +} diff --git a/src/views/check/results/failedFileRequest.html b/src/views/check/results/failedFileRequest.html index bd1888177..279e8aa88 100644 --- a/src/views/check/results/failedFileRequest.html +++ b/src/views/check/results/failedFileRequest.html @@ -19,7 +19,7 @@
If you typed the web address, check it is correct.
- -If you pasted the web address, check you copied the entire address.
- -If the web address is correct or you selected a link or button send an email - to digitalland@communities.gov.uk.
- -If you typed the web address, check it is correct.
- -If you pasted the web address, check you copied the entire address.
- -If the web address is correct or you selected a link or button send an email - to digitalland@communities.gov.uk.
- -The service will be unavailable from {{downtime}}.
- {% endif %} - {% if upTime %} -You’ll be able to use the service from {{upTime}}.
- {% endif %} -If you typed the web address, check it is correct.
+If you pasted the web address, check you copied the entire address.
+If the web address is correct or you selected a link or button send an email to {{supportEmail}}.
+ + {% elif err.statusCode === 404 %} +If you typed the address, check it is correct.
+If you copied the web address, check you copied the entire address.
+If the web address is correct or you selected a link or button, send an email to {{supportEmail}} for help.
+ +If you used an old address, it's possible it has expired. You can start the process from the beginning.
+ + {% elif err.statusCode === 503 %} + {% if downtime %} +The service will be unavailable from {{downtime}}.
+ {% endif %} + {% if uptime %} +You’ll be able to use the service from {{uptime}}.
+ {% endif %} + + {% else %} +Try again later.
+You'll need to start from the beginning of the service.
+ {% endif %} + ++ {{ err.stack | safe }} ++ {% if err.cause %} +
Cause:
++ {{ err.cause.stack | safe }} ++ {% endif %} +
Path: [{{ issue.path.join(', ') }}]
+Message:: {{ issue.message }}
+