Merge pull request #118 from Tietokilta/feat/useful-error-messages

Useful error messages

packages/ilmomasiina-backend/package.json

@@ -41,6 +41,8 @@
     "@fastify/type-provider-typebox": "^2.3.0",
     "@sinclair/typebox": "^0.24.22",
     "@tietokilta/ilmomasiina-models": "workspace:2.0.0-alpha13",
+    "ajv": "^8.12.0",
+    "ajv-formats": "^2.1.1",
     "base32-encode": "^1.2.0",
     "bcrypt": "^5.1.0",
     "better-npm-run": "^0.1.1",

packages/ilmomasiina-backend/src/app.ts

@@ -2,6 +2,8 @@ import fastifyCompress from '@fastify/compress';
 import fastifyCors from '@fastify/cors';
 import fastifySensible from '@fastify/sensible';
 import fastifyStatic from '@fastify/static';
+import Ajv from 'ajv';
+import ajvFormats from 'ajv-formats';
 import fastify, { FastifyInstance } from 'fastify';
 import cron from 'node-cron';
 import path from 'path';
@@ -16,18 +18,35 @@ import enforceHTTPS from './enforceHTTPS';
 import setupDatabase from './models';
 import setupRoutes from './routes';
 
+// Disable type coercion for request bodies - we don't need it, and it breaks stuff like anyOf
+const bodyCompiler = new Ajv({
+  coerceTypes: false,
+  useDefaults: true,
+  removeAdditional: true,
+  addUsedSchema: false,
+  allErrors: false,
+});
+ajvFormats(bodyCompiler);
+
+const defaultCompiler = new Ajv({
+  coerceTypes: 'array',
+  useDefaults: true,
+  removeAdditional: true,
+  addUsedSchema: false,
+  allErrors: false,
+});
+ajvFormats(defaultCompiler);
+
 export default async function initApp(): Promise<FastifyInstance> {
   await setupDatabase();
 
   const server = fastify({
     trustProxy: config.isAzure || config.trustProxy, // Get IPs from X-Forwarded-For
     logger: true, // Enable logger
-    ajv: {
-      customOptions: {
-        coerceTypes: false, // Disable type coercion - we don't need it, and it breaks stuff like anyOf
-      },
-    },
   });
+  server.setValidatorCompiler(({ httpPart, schema }) => (
+    httpPart === 'body' ? bodyCompiler.compile(schema) : defaultCompiler.compile(schema)
+  ));
 
   // Register fastify-sensible (https://github.com/fastify/fastify-sensible)
   server.register(fastifySensible);

packages/ilmomasiina-backend/src/authentication/adminAuthSession.ts

@@ -4,6 +4,7 @@ import {
 import { FastifyRequest } from 'fastify';
 
 import type { UserID, UserSchema } from '@tietokilta/ilmomasiina-models';
+import config from '../config';
 import { BadSession } from './errors';
 
 export interface AdminTokenData {
@@ -13,7 +14,7 @@ export interface AdminTokenData {
 
 export default class AdminAuthSession {
   /** Session lifetime in seconds */
-  static TTL = 10 * 60;
+  static TTL = config.nodeEnv === 'development' ? 365 * 24 * 60 * 60 : 10 * 60;
 
   private readonly sign: typeof SignerSync;
   private readonly verify: typeof VerifierSync;

packages/ilmomasiina-backend/src/routes/index.ts

@@ -18,10 +18,7 @@ import deleteUser from './admin/users/deleteUser';
 import listUsers from './admin/users/listUsers';
 import resetPassword from './admin/users/resetPassword';
 import { adminLogin, requireAdmin } from './authentication/adminLogin';
-import {
-  getEventDetailsForAdmin,
-  getEventDetailsForUser,
-} from './events/getEventDetails';
+import { getEventDetailsForAdmin, getEventDetailsForUser } from './events/getEventDetails';
 import { getEventsListForAdmin, getEventsListForUser } from './events/getEventsList';
 import { sendICalFeed } from './ical';
 import createSignup from './signups/createNewSignup';

packages/ilmomasiina-backend/src/routes/signups/createNewSignup.ts

@@ -1,12 +1,12 @@
 import { FastifyReply, FastifyRequest } from 'fastify';
-import { Forbidden, NotFound } from 'http-errors';
 
 import type { SignupCreateBody, SignupCreateResponse } from '@tietokilta/ilmomasiina-models';
 import { Event } from '../../models/event';
 import { Quota } from '../../models/quota';
 import { Signup } from '../../models/signup';
 import { refreshSignupPositions } from './computeSignupPosition';
 import { generateToken } from './editTokens';
+import { NoSuchQuota, SignupsClosed } from './errors';
 
 export const signupsAllowed = (event: Event) => {
   if (event.registrationStartDate === null || event.registrationEndDate === null) {
@@ -34,11 +34,11 @@ export default async function createSignup(
 
   // Do some validation.
   if (!quota) {
-    throw new NotFound('Quota doesn\'t exist.');
+    throw new NoSuchQuota('Quota doesn\'t exist.');
   }
 
   if (!signupsAllowed(quota.event!)) {
-    throw new Forbidden('Signups closed for this event.');
+    throw new SignupsClosed('Signups closed for this event.');
   }
 
   // Create the signup.

packages/ilmomasiina-backend/src/routes/signups/deleteSignup.ts

@@ -1,5 +1,4 @@
 import { FastifyReply, FastifyRequest } from 'fastify';
-import { Forbidden, NotFound } from 'http-errors';
 
 import type { SignupPathParams } from '@tietokilta/ilmomasiina-models';
 import { AuditEvent } from '@tietokilta/ilmomasiina-models';
@@ -9,6 +8,7 @@ import { Quota } from '../../models/quota';
 import { Signup } from '../../models/signup';
 import { refreshSignupPositions } from './computeSignupPosition';
 import { signupsAllowed } from './createNewSignup';
+import { NoSuchSignup, SignupsClosed } from './errors';
 
 /** Requires admin authentication OR editTokenVerification */
 async function deleteSignup(id: string, auditLogger: AuditLogger, admin: boolean = false): Promise<void> {
@@ -29,10 +29,10 @@ async function deleteSignup(id: string, auditLogger: AuditLogger, admin: boolean
       transaction,
     });
     if (signup === null) {
-      throw new NotFound('No signup found with id');
+      throw new NoSuchSignup('No signup found with id');
     }
     if (!admin && !signupsAllowed(signup.quota!.event!)) {
-      throw new Forbidden('Signups closed for this event.');
+      throw new SignupsClosed('Signups closed for this event.');
     }
 
     // Delete the DB object

packages/ilmomasiina-backend/src/routes/signups/editTokens.ts

@@ -1,10 +1,11 @@
 import base32Encode from 'base32-encode';
 import { createHash, createHmac } from 'crypto';
-import { FastifyReply, FastifyRequest } from 'fastify';
+import { FastifyRequest } from 'fastify';
 
 import type { SignupID, SignupPathParams } from '@tietokilta/ilmomasiina-models';
-import { EDIT_TOKEN_HEADER } from '@tietokilta/ilmomasiina-models';
+import { EDIT_TOKEN_HEADER, ErrorCode } from '@tietokilta/ilmomasiina-models';
 import config from '../../config';
+import CustomError from '../../util/customError';
 
 function generateLegacyToken(signupId: SignupID): string {
   const data = Buffer.from(`${signupId}${config.oldEditTokenSalt}`, 'utf-8');
@@ -28,21 +29,24 @@ function verifyToken(signupId: SignupID, token: string): boolean {
   return token === expectedToken;
 }
 
+class BadEditToken extends CustomError {
+  constructor(message: string) {
+    super(403, ErrorCode.BAD_EDIT_TOKEN, message);
+  }
+}
+
 /**
  * A preHandler hook that validates signup edit token
  *
  * When the token is not valid, replies with a 403 request with a generic `invalid token`-like error message.
  * The request processing ends here, and the actual route function won't be called.
  */
-export async function requireValidEditToken(
-  request: FastifyRequest<{ Params: SignupPathParams }>,
-  reply: FastifyReply,
-): Promise<void> {
+export async function requireValidEditToken(request: FastifyRequest<{ Params: SignupPathParams }>): Promise<void> {
   // Fastify converts header names into lower case
   const headers = request.headers[EDIT_TOKEN_HEADER.toLowerCase()];
   const header = Array.isArray(headers) ? headers[0] : headers;
   if (verifyToken(request.params.id, header || '')) return;
 
   // Default to 403 response
-  reply.forbidden('Invalid editToken');
+  throw new BadEditToken('Invalid editToken');
 }

packages/ilmomasiina-backend/src/routes/signups/errors.ts

@@ -0,0 +1,21 @@
+/* eslint-disable max-classes-per-file */
+import { ErrorCode } from '@tietokilta/ilmomasiina-models';
+import CustomError from '../../util/customError';
+
+export class SignupsClosed extends CustomError {
+  constructor(message: string) {
+    super(403, ErrorCode.SIGNUPS_CLOSED, message);
+  }
+}
+
+export class NoSuchQuota extends CustomError {
+  constructor(message: string) {
+    super(404, ErrorCode.NO_SUCH_QUOTA, message);
+  }
+}
+
+export class NoSuchSignup extends CustomError {
+  constructor(message: string) {
+    super(404, ErrorCode.NO_SUCH_SIGNUP, message);
+  }
+}

packages/ilmomasiina-backend/src/routes/signups/getSignupForEdit.ts

@@ -1,5 +1,4 @@
 import { FastifyReply, FastifyRequest } from 'fastify';
-import { NotFound } from 'http-errors';
 
 import type { SignupForEditResponse, SignupPathParams } from '@tietokilta/ilmomasiina-models';
 import { Answer } from '../../models/answer';
@@ -8,6 +7,7 @@ import { Question } from '../../models/question';
 import { Quota } from '../../models/quota';
 import { Signup } from '../../models/signup';
 import { stringifyDates } from '../utils';
+import { NoSuchSignup } from './errors';
 
 /** Requires editTokenVerification */
 export default async function getSignupForEdit(
@@ -39,7 +39,7 @@ export default async function getSignupForEdit(
   });
   if (signup === null) {
     // Event not found with id, probably deleted
-    throw new NotFound('No signup found with given id');
+    throw new NoSuchSignup('No signup found with given id');
   }
 
   const event = signup.quota!.event!;

packages/ilmomasiina-backend/src/routes/signups/updateSignup.ts

@@ -1,5 +1,5 @@
 import { FastifyReply, FastifyRequest } from 'fastify';
-import { BadRequest, Forbidden, NotFound } from 'http-errors';
+import { BadRequest } from 'http-errors';
 import { Transaction } from 'sequelize';
 
 import type { SignupPathParams, SignupUpdateBody, SignupUpdateResponse } from '@tietokilta/ilmomasiina-models';
@@ -10,6 +10,7 @@ import { Event } from '../../models/event';
 import { Question } from '../../models/question';
 import { Signup } from '../../models/signup';
 import { signupsAllowed } from './createNewSignup';
+import { NoSuchSignup, SignupsClosed } from './errors';
 
 /** Requires editTokenVerification */
 export default async function updateSignup(
@@ -24,7 +25,7 @@ export default async function updateSignup(
       lock: Transaction.LOCK.UPDATE,
     });
     if (signup === null) {
-      throw new NotFound('Signup expired or already deleted');
+      throw new NoSuchSignup('Signup expired or already deleted');
     }
 
     const quota = await signup.getQuota({
@@ -42,7 +43,7 @@ export default async function updateSignup(
     });
     const event = quota.event!;
     if (!signupsAllowed(event)) {
-      throw new Forbidden('Signups closed for this event.');
+      throw new SignupsClosed('Signups closed for this event.');
     }
 
     /** Is this signup already confirmed (i.e. is this the first update for this signup) */

packages/ilmomasiina-components/package.json

@@ -26,13 +26,13 @@
   },
   "dependencies": {
     "@tietokilta/ilmomasiina-models": "workspace:2.0.0-alpha13",
-    "@types/lodash": "^4.14.182",
+    "@types/lodash-es": "^4.17.9",
     "@types/react": "^17 || ^18",
     "bootstrap": "^4.6.1",
     "final-form": "^4.20.10",
     "formik": "^2.4.5",
     "i18next": "^22.4.11",
-    "lodash": "^4.17.21",
+    "lodash-es": "^4.17.21",
     "moment": "^2.29.3",
     "moment-timezone": "^0.5.34",
     "react": "^17 || ^18",

packages/ilmomasiina-components/src/api.ts

@@ -22,16 +22,15 @@ export class ApiError extends Error {
   }
 
   static async fromResponse(response: Response) {
-    let error = new Error(response.statusText);
     try {
       const data = await response.json();
       if (data.message) {
-        error = new ApiError(response.status, data);
+        return new ApiError(response.status, data);
       }
     } catch (e) {
       /* fall through */
     }
-    return error;
+    return new ApiError(response.status, { message: response.statusText });
   }
 }
 
@@ -59,10 +58,20 @@ export default async function apiFetch(uri: string, {
     body: body === undefined ? undefined : JSON.stringify(body),
     headers: allHeaders,
     signal,
+  }).catch((err) => {
+    // convert network errors to barebones ApiError
+    throw new ApiError(0, err);
   });
-
+  // proper API errors, try to parse JSON
   if (!response.ok) {
     throw await ApiError.fromResponse(response);
   }
-  return response.status === 204 ? null : response.json();
+  // 204 No Content
+  if (response.status === 204) {
+    return null;
+  }
+  // just in case, convert JSON parse errors for 2xx responses to ApiError
+  return response.json().catch((err) => {
+    throw new ApiError(0, err);
+  });
 }

packages/ilmomasiina-components/src/i18n.tsx

@@ -16,6 +16,7 @@ export { resources as i18nResources };
 const i18n = createInstance({
   resources,
   fallbackLng: 'fi',
+  defaultNS: 'components',
   returnNull: false,
   interpolation: {
     // for React