From d224f1444d2413d6f126d00565aee9460eb1b29b Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Wed, 1 Apr 2026 12:40:57 -0500 Subject: [PATCH 1/6] Improve route error handling consistency --- UPSTREAM_CANDIDATES.md | 54 ++++++++++++++++++++++++ error-messenger.js | 39 +++++++++++------ rest.js | 37 ++++++++++++++++ routes/__tests__/create.test.js | 39 ++++++++++++++++- routes/__tests__/error-messenger.test.js | 30 +++++++++++++ routes/__tests__/query.test.js | 42 +++++++++++++++++- routes/create.js | 7 +-- routes/delete.js | 20 +++++---- routes/overwrite.js | 8 ++-- routes/query.js | 26 ++++++++---- routes/update.js | 7 +-- 11 files changed, 269 insertions(+), 40 deletions(-) create mode 100644 UPSTREAM_CANDIDATES.md create mode 100644 rest.js create mode 100644 routes/__tests__/error-messenger.test.js diff --git a/UPSTREAM_CANDIDATES.md b/UPSTREAM_CANDIDATES.md new file mode 100644 index 0000000..095ec9d --- /dev/null +++ b/UPSTREAM_CANDIDATES.md @@ -0,0 +1,54 @@ +# TinyPen to TinyNode Upstream Candidates + +This document tracks candidate updates identified from TinyPen that may be upstreamed into TinyNode. + +## Summary + +- Inclusion policy: high-confidence candidates plus uncertainty items. +- Evidence policy: each candidate must include TinyPen evidence references. +- Delivery policy: multiple focused PRs by theme. + +## Candidate Matrix + +| ID | Candidate | Status | TinyPen Evidence | TinyNode Targets | Notes | +| --- | --- | --- | --- | --- | --- | +| C-001 | Content-Type validation middleware for JSON endpoints | upstream-now | PR #39 | `routes/query.js`, `routes/create.js`, `routes/update.js`, `routes/delete.js`, `routes/overwrite.js` | Detect malformed or unsupported media types and return 415. | +| C-002 | Query body validation for empty object/array | upstream-now | PR #33 | `routes/query.js`, `routes/__tests__/query.test.js` | Reject empty query payloads with 400. | +| C-003 | Query pagination validation for `limit` and `skip` | upstream-now | PR #33 | `routes/query.js`, `routes/__tests__/query.test.js` | Require non-negative integers when provided. | +| C-004 | Route-level error semantic consistency | upstream-now | PR #39 and related | `routes/*.js`, `error-messenger.js` | Partial implementation landed: explicit 502 network failures, structured JSON passthrough, and fixed upstream propagation defects. Remaining route-specific parity can follow later. | +| C-005 | Dependency cleanup after Node upgrade | investigate-first | TinyPen dependency diffs | `package.json` | Verify runtime paths before removing deps (`morgan`, `dotenv-expand`, `jsonld`). | +| C-006 | Optimistic locking parity improvements | investigate-first | PR #20 | `routes/overwrite.js`, `routes/__tests__/overwrite.test.js` | Keep current behavior, evaluate if more parity is needed. | +| C-007 | Temporary test alignment imports from TinyPen | upstream-now | PR #35 context | `routes/__tests__/*` | Include useful generated coverage temporarily while test strategy evolves. | + +## Excluded or Deferred + +| ID | Item | Reason | +| --- | --- | --- | +| X-001 | `.claude` and Claude-specific workflow changes | TPEN-internal tooling. | +| X-002 | TinyPen naming substitutions in API messages | TinyNode and TinyPen naming is intentionally distinct for disambiguation. | +| X-003 | Test framework architecture migration | Out of current scope. | + +## PR Grouping Proposal + +1. PR-A: request/content validation guards (`C-001`, `C-002`, `C-003`) +2. PR-B: error semantic consistency (`C-004`) +3. PR-C: cleanup and dependency simplification (`C-005`) +4. PR-D: temporary test alignment updates (`C-007`) + +## Verification Gates + +1. Each merged candidate row includes evidence reference and touched file list. +2. Targeted route tests pass for touched modules. +3. Full test suite passes before merge. +4. Dependency changes include runtime verification notes. +5. Uncertain candidates remain blocked until evidence is upgraded from investigate-first to upstream-now. + +## Deferred Test Cases + +These scenarios are worth adding as follow-up coverage, but do not block the current implementation slice. + +1. `create`, `update`, `delete`, and `overwrite` should each preserve upstream text error bodies and status codes. +2. `update`, `delete`, and `overwrite` should each map network failures to `502 Bad Gateway`. +3. `overwrite` should preserve its special `409` JSON conflict response when `If-Overwritten-Version` negotiation fails. +4. `delete/:id` should verify upstream non-2xx responses are propagated through the shared messenger instead of failing locally. +5. Shared error handling should verify fallback behavior for generic thrown `Error` objects without upstream response metadata. diff --git a/error-messenger.js b/error-messenger.js index acaa354..94d12cb 100644 --- a/error-messenger.js +++ b/error-messenger.js @@ -12,22 +12,37 @@ export async function messenger(rerum_error_res, req, res, next) { if (res.headersSent) { return } - let error = {} - let rerum_err_text + + const error = { + message: rerum_error_res.statusMessage ?? rerum_error_res.message ?? "A server error has occured", + status: rerum_error_res.statusCode ?? rerum_error_res.status ?? 500, + body: rerum_error_res.body + } + + if (error.body !== undefined) { + console.error(error) + res.status(error.status).json(error.body) + return + } + try { - // Unless already handled upstream the rerum_error_res is an error Response with details as a textual body. - rerum_err_text = await rerum_error_res.text() + const contentType = rerum_error_res.headers?.get?.("Content-Type")?.toLowerCase?.() ?? "" + if (contentType.includes("json")) { + error.body = await rerum_error_res.json() + console.error(error) + res.status(error.status).json(error.body) + return + } + + const rerumErrText = await rerum_error_res.text() + if (rerumErrText) { + error.message = rerumErrText + } } catch (err) { - // It is some 500 - rerum_err_text = undefined - } - if (rerum_err_text) error.message = rerum_err_text - else { - // Perhaps this is a more generic 500 from the app, perhaps involving RERUM, and there is no good rerum_error_res - error.message = rerum_error_res.statusMessage ?? rerum_error_res.message ?? `A server error has occured` + // Fall back to the status/message values already collected above. } - error.status = rerum_error_res.statusCode ?? rerum_error_res.status ?? 500 + console.error(error) res.set("Content-Type", "text/plain; charset=utf-8") res.status(error.status).send(error.message) diff --git a/rest.js b/rest.js new file mode 100644 index 0000000..23c45d3 --- /dev/null +++ b/rest.js @@ -0,0 +1,37 @@ +const acceptedJsonContentTypes = new Set([ + "application/json", + "application/ld+json" +]) + +export function httpError(message, status = 500, body) { + const error = new Error(message) + error.status = status + if (body !== undefined) { + error.body = body + } + return error +} + +function getContentType(req) { + const rawContentType = req.headers?.["content-type"] + if (!rawContentType) { + return "" + } + if (rawContentType.includes(",")) { + return "multiple" + } + return rawContentType.split(";")[0].trim().toLowerCase() +} + +export function verifyJsonContentType(req, res, next) { + const contentType = getContentType(req) + if (contentType === "multiple") { + res.status(415).send("Unsupported Media Type. Multiple Content-Type values are not allowed.") + return + } + if (acceptedJsonContentTypes.has(contentType)) { + next() + return + } + res.status(415).send("Unsupported Media Type. Use application/json or application/ld+json.") +} diff --git a/routes/__tests__/create.test.js b/routes/__tests__/create.test.js index e928820..b9f47cc 100644 --- a/routes/__tests__/create.test.js +++ b/routes/__tests__/create.test.js @@ -3,6 +3,7 @@ import request from "supertest" import { jest } from "@jest/globals" import createRoute from "../create.js" +import { messenger } from "../../error-messenger.js" //import app from "../../app.js" const routeTester = new express() @@ -10,6 +11,7 @@ routeTester.use(express.json()) routeTester.use(express.urlencoded({ extended: false })) routeTester.use("/create", createRoute) routeTester.use("/app/create", createRoute) +routeTester.use(messenger) const rerum_uri = `${process.env.RERUM_ID_PATTERN}_not_` @@ -108,6 +110,41 @@ describe("Check that incorrect TinyNode create route usage results in expected R }) }) +describe("Check that TinyNode create route propagates upstream and network errors predictably. __rest __core", () => { + it("Preserves upstream text errors and maps network failures to 502.", async () => { + global.fetch = jest.fn(() => + Promise.resolve({ + ok: false, + status: 503, + headers: { + get: () => "text/plain; charset=utf-8" + }, + text: () => Promise.resolve("Upstream create failure") + }) + ) + + let response = await request(routeTester) + .post("/create") + .set("Content-Type", "application/json") + .send({ "test": "item" }) + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(503) + expect(response.text).toBe("Upstream create failure") + + global.fetch = jest.fn(() => Promise.reject(new Error("socket hang up"))) + + response = await request(routeTester) + .post("/create") + .set("Content-Type", "application/json") + .send({ "test": "item" }) + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(502) + expect(response.text).toContain("socket hang up") + }) +}) + /** * Full integration test. Checks the TinyNode app create endpoint functionality and RERUM connection. * @@ -125,4 +162,4 @@ describe("Check that the properly used create endpoints function and interact wi expect(response.statusCode).toBe(201) expect(response.body.test).toBe("item") }) -}) \ No newline at end of file +}) diff --git a/routes/__tests__/error-messenger.test.js b/routes/__tests__/error-messenger.test.js new file mode 100644 index 0000000..5e70992 --- /dev/null +++ b/routes/__tests__/error-messenger.test.js @@ -0,0 +1,30 @@ +import express from "express" +import request from "supertest" +import { messenger } from "../../error-messenger.js" + +const app = express() + +app.get("/json-error", (req, res, next) => { + next({ + status: 422, + headers: { + get: () => "application/json" + }, + json: async () => ({ message: "Invalid payload", field: "name" }) + }) +}) + +app.use(messenger) + +describe("Check shared error messenger behavior. __rest __core", () => { + it("Returns structured JSON error bodies when upstream responds with JSON.", async () => { + const response = await request(app) + .get("/json-error") + .then(resp => resp) + .catch(err => err) + + expect(response.statusCode).toBe(422) + expect(response.body.message).toBe("Invalid payload") + expect(response.body.field).toBe("name") + }) +}) diff --git a/routes/__tests__/query.test.js b/routes/__tests__/query.test.js index 3b62cd4..bc8bac0 100644 --- a/routes/__tests__/query.test.js +++ b/routes/__tests__/query.test.js @@ -95,6 +95,46 @@ describe("Check that incorrect TinyNode query route usage results in expected RE .catch(err => err) expect(response.statusCode).toBe(400) + response = await request(routeTester) + .post("/query") + .set("Content-Type", "application/json") + .send({}) + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(400) + + response = await request(routeTester) + .post("/query") + .set("Content-Type", "application/json") + .send([]) + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(400) + + response = await request(routeTester) + .post("/query?limit=-1") + .set("Content-Type", "application/json") + .send({ "test": "item" }) + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(400) + + response = await request(routeTester) + .post("/query?skip=abc") + .set("Content-Type", "application/json") + .send({ "test": "item" }) + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(400) + + response = await request(routeTester) + .post("/query") + .set("Content-Type", "text/plain") + .send("plain text") + .then(resp => resp) + .catch(err => err) + expect(response.statusCode).toBe(415) + }) }) @@ -114,4 +154,4 @@ describe("Check that the properly used query endpoints function and interact wit expect(response.statusCode).toBe(200) expect(response.body[0].test).toBe("item") }) -}) \ No newline at end of file +}) diff --git a/routes/create.js b/routes/create.js index 23056f4..cac8940 100644 --- a/routes/create.js +++ b/routes/create.js @@ -1,9 +1,10 @@ import express from "express" import checkAccessToken from "../tokens.js" +import { httpError, verifyJsonContentType } from "../rest.js" const router = express.Router() /* POST a create to the thing. */ -router.post('/', checkAccessToken, async (req, res, next) => { +router.post('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { // check body for JSON @@ -25,7 +26,7 @@ router.post('/', checkAccessToken, async (req, res, next) => { return res }) .catch(err => { - throw err + throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) }) // Send RERUM error responses to error-messenger.js if (errored) return next(result) @@ -42,4 +43,4 @@ router.all('/', (req, res, next) => { res.status(405).send("Method Not Allowed") }) -export default router \ No newline at end of file +export default router diff --git a/routes/delete.js b/routes/delete.js index a3e80a1..d9778a9 100644 --- a/routes/delete.js +++ b/routes/delete.js @@ -1,11 +1,12 @@ import express from "express" import checkAccessToken from "../tokens.js" +import { httpError, verifyJsonContentType } from "../rest.js" const router = express.Router() /* Legacy delete pattern w/body */ /* DELETE a delete to the thing. */ -router.delete('/', checkAccessToken, async (req, res, next) => { +router.delete('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { // check for @id in body. Any value is valid. Lack of value is a bad request. if (!req?.body || !(req.body['@id'] ?? req.body.id)) { @@ -24,15 +25,18 @@ router.delete('/', checkAccessToken, async (req, res, next) => { } const deleteURL = `${process.env.RERUM_API_ADDR}delete` let errored = false - const result = await fetch(deleteURL, deleteOptions).then(res=>{ - if (!res.ok) errored = true + const result = await fetch(deleteURL, deleteOptions).then(async res=>{ + if (!res.ok) { + errored = true + return res + } return res.text() }) .catch(err => { - throw err + throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) }) // Send RERUM error responses to error-messenger.js - if (errored) return next(results) + if (errored) return next(result) res.status(204) res.send(result) } @@ -55,14 +59,14 @@ router.delete('/:id', async (req, res, next) => { } let errored = false const result = await fetch(deleteURL, deleteOptions).then(res => { - if(!res.ok) errored = true + if (!res.ok) errored = true return res }) .catch(err => { - throw err + throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) }) // Send RERUM error responses to error-messenger.js - if (errored) return next(results) + if (errored) return next(result) res.status(204) res.send(result) } diff --git a/routes/overwrite.js b/routes/overwrite.js index b2bb6dd..d09dcac 100644 --- a/routes/overwrite.js +++ b/routes/overwrite.js @@ -1,9 +1,10 @@ import express from "express" import checkAccessToken from "../tokens.js" +import { httpError, verifyJsonContentType } from "../rest.js" const router = express.Router() /* PUT an overwrite to the thing. */ -router.put('/', checkAccessToken, async (req, res, next) => { +router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { @@ -42,7 +43,8 @@ router.put('/', checkAccessToken, async (req, res, next) => { .then(async rerum_res=>{ if (rerum_res.ok) return rerum_res.json() errored = true - if (rerum_res.headers.get("Content-Type").includes("json")) { + const contentType = rerum_res.headers.get("Content-Type")?.toLowerCase() ?? "" + if (contentType.includes("json")) { // Special handling. This does not go through to error-messenger.js if (rerum_res.status === 409) { const currentVersion = await rerum_res.json() @@ -52,7 +54,7 @@ router.put('/', checkAccessToken, async (req, res, next) => { return rerum_res }) .catch(err => { - throw err + throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) }) // Send RERUM error responses to error-messenger.js if (errored) return next(response) diff --git a/routes/query.js b/routes/query.js index 4b06a82..a2a69e5 100644 --- a/routes/query.js +++ b/routes/query.js @@ -1,20 +1,28 @@ import express from "express" +import { verifyJsonContentType } from "../rest.js" const router = express.Router() /* POST a query to the thing. */ -router.post('/', async (req, res, next) => { +router.post('/', verifyJsonContentType, async (req, res, next) => { const lim = req.query.limit ?? 10 const skip = req.query.skip ?? 0 + const limit = Number.parseInt(lim, 10) + const offset = Number.parseInt(skip, 10) try { - // check body for JSON - const body = JSON.stringify(req.body) - // check limit and skip for INT - if (isNaN(parseInt(lim) + parseInt(skip)) - || (lim < 0) - || (skip < 0)) { - throw Error("`limit` and `skip` values must be positive integers or omitted.") + if (!req.body || typeof req.body !== "object" || Array.isArray(req.body)) { + res.status(400).send("Query payload must be a non-empty JSON object.") + return + } + if (Object.keys(req.body).length === 0) { + res.status(400).send("Query payload must not be an empty object.") + return } + if (!Number.isInteger(limit) || !Number.isInteger(offset) || limit < 0 || offset < 0) { + res.status(400).send("`limit` and `skip` values must be non-negative integers or omitted.") + return + } + const body = JSON.stringify(req.body) const queryOptions = { method: 'POST', body, @@ -24,7 +32,7 @@ router.post('/', async (req, res, next) => { 'Content-Type' : "application/json;charset=utf-8" } } - const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${lim}&skip=${skip}` + const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${limit}&skip=${offset}` let errored = false const results = await fetch(queryURL, queryOptions).then(res=>{ if (res.ok) return res.json() diff --git a/routes/update.js b/routes/update.js index b5ca5d5..5956f68 100644 --- a/routes/update.js +++ b/routes/update.js @@ -1,9 +1,10 @@ import express from "express" import checkAccessToken from "../tokens.js" +import { httpError, verifyJsonContentType } from "../rest.js" const router = express.Router() /* PUT an update to the thing. */ -router.put('/', checkAccessToken, async (req, res, next) => { +router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { // check for @id in body. Any value is valid. Lack of value is a bad request. @@ -30,13 +31,13 @@ router.put('/', checkAccessToken, async (req, res, next) => { return res }) .catch(err => { - throw err + throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) }) // Send RERUM error responses to error-messenger.js if (errored) return next(result) res.setHeader("Location", result["@id"] ?? result.id) res.status(200) - res.send(result) + res.json(result) } catch (err) { next(err) From cb94cb6992b6f8260616fc9f8a35078d569c7382 Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Thu, 2 Apr 2026 09:19:22 -0500 Subject: [PATCH 2/6] Add RERUM fetch helper; improve route errors Introduce rerum.js with fetchRerum (timeout/abort handling) and helpers for network/timeout errors. Update create, update, overwrite, delete and query routes to use fetchRerum, add Origin header and unified error handling, validate request bodies/params more strictly, and surface RERUM textual error bodies as 502 responses. Also handle id/_id normalization, reject empty queries, and specially return 409 conflict bodies from overwrite. --- rerum.js | 62 +++++++++++++++++++++++++++++++++++ routes/create.js | 49 ++++++++++++++++----------- routes/delete.js | 80 +++++++++++++++++++++++++-------------------- routes/overwrite.js | 67 ++++++++++++++++++++----------------- routes/query.js | 66 +++++++++++++++++++++---------------- routes/update.js | 55 ++++++++++++++++++------------- 6 files changed, 243 insertions(+), 136 deletions(-) create mode 100644 rerum.js diff --git a/rerum.js b/rerum.js new file mode 100644 index 0000000..0eb3a1e --- /dev/null +++ b/rerum.js @@ -0,0 +1,62 @@ +const DEFAULT_RERUM_TIMEOUT_MS = 30000 + +const getRerumTimeoutMs = () => { + const timeoutMs = Number.parseInt(process.env.RERUM_FETCH_TIMEOUT_MS ?? `${DEFAULT_RERUM_TIMEOUT_MS}`, 10) + return Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : DEFAULT_RERUM_TIMEOUT_MS +} + +/** + * Build the generic upstream error used when RERUM cannot be reached or returns invalid data. + * + * @param {string} url - The RERUM URL being requested + * @returns {Error} + */ +const createRerumNetworkError = (url) => { + const err = new Error(`500: ${url} - A RERUM error occurred`) + err.status = 502 + return err +} + +/** + * Build an upstream timeout error for requests that exceed the configured wait time. + * + * @param {string} url - The RERUM URL being requested + * @param {number} timeoutMs - The timeout in milliseconds + * @returns {Error} + */ +const createRerumTimeoutError = (url, timeoutMs) => { + const err = new Error(`504: ${url} - RERUM did not respond within ${timeoutMs}ms`) + err.status = 504 + return err +} + +/** + * Execute a fetch to RERUM with a bounded wait time so workers do not block indefinitely. + * + * @param {string} url - The RERUM URL being requested + * @param {RequestInit} [options={}] - Fetch options + * @returns {Promise} + */ +async function fetchRerum(url, options = {}) { + const timeoutMs = getRerumTimeoutMs() + const timeoutController = new AbortController() + const timeoutId = setTimeout(() => timeoutController.abort(), timeoutMs) + const signal = options.signal + ? AbortSignal.any([options.signal, timeoutController.signal]) + : timeoutController.signal + + try { + return await fetch(url, { ...options, signal }) + } + catch (err) { + if (err?.name === "AbortError" && timeoutController.signal.aborted) { + throw createRerumTimeoutError(url, timeoutMs) + } + throw createRerumNetworkError(url) + } + finally { + clearTimeout(timeoutId) + } +} + +export { createRerumNetworkError, fetchRerum } diff --git a/routes/create.js b/routes/create.js index b86d539..4bc03cc 100644 --- a/routes/create.js +++ b/routes/create.js @@ -1,44 +1,55 @@ import express from "express" import checkAccessToken from "../tokens.js" -import { httpError, verifyJsonContentType } from "../rest.js" +import { verifyJsonContentType } from "../rest.js" +import { createRerumNetworkError, fetchRerum } from "../rerum.js" const router = express.Router() /* POST a create to the thing. */ router.post('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { + // if an id is passed in, pop off the end to make it an _id + if (req.body.id) { + req.body._id = req.body.id.split('/').pop() + } + // check body for JSON - const body = JSON.stringify(req.body) + const createBody = JSON.stringify(req.body) const createOptions = { method: 'POST', - body, + body: createBody, headers: { 'user-agent': 'Tiny-Things/1.0', + 'Origin': process.env.ORIGIN, 'Authorization': `Bearer ${process.env.ACCESS_TOKEN}`, 'Content-Type' : "application/json;charset=utf-8" } } const createURL = `${process.env.RERUM_API_ADDR}create` - let errored = false - const result = await fetch(createURL, createOptions).then(res=>{ - if (res.ok) return res.json() - errored = true - return res - }) - .catch(err => { - throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) + const rerumResponse = await fetchRerum(createURL, createOptions) + .then(async (resp) => { + if (resp.ok) return resp.json() + // The response from RERUM indicates a failure, likely with a specific code and textual body + let rerumErrorMessage + try { + rerumErrorMessage = `${resp.status ?? 500}: ${createURL} - ${await resp.text()}` + } catch (e) { + rerumErrorMessage = `500: ${createURL} - A RERUM error occurred` + } + const err = new Error(rerumErrorMessage) + err.status = 502 + throw err }) - // Send RERUM error responses to error-messenger.js - if (errored) return next(result) - const location = result?.["@id"] ?? result?.id - const responseBody = { ...req.body, ...(result ?? {}) } - if (location) { - res.setHeader("Location", location) + if (!(rerumResponse.id || rerumResponse["@id"])) { + // A 200 with garbled data, call it a fail + throw createRerumNetworkError(createURL) } - res.status(201).json(responseBody) + res.setHeader("Location", rerumResponse["@id"] ?? rerumResponse.id) + res.status(201).json(rerumResponse) } catch (err) { - next(err) + console.error(err) + res.status(err.status ?? 500).type('text/plain').send(err.message ?? 'An error occurred') } }) diff --git a/routes/delete.js b/routes/delete.js index 74637d6..4bc3912 100644 --- a/routes/delete.js +++ b/routes/delete.js @@ -1,75 +1,83 @@ import express from "express" import checkAccessToken from "../tokens.js" -import { httpError, verifyJsonContentType } from "../rest.js" +import { verifyJsonContentType } from "../rest.js" +import { fetchRerum } from "../rerum.js" const router = express.Router() -/* Legacy delete pattern w/body */ - -/* DELETE a delete to the thing. */ +/* Legacy delete pattern w/body. */ router.delete('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { - // check for @id in body. Any value is valid. Lack of value is a bad request. if (!req?.body || !(req.body['@id'] ?? req.body.id)) { - res.status(400).send("No record id to delete! (https://store.rerum.io/v1/API.html#delete)") - return + const err = new Error("No record id to delete! (https://store.rerum.io/v1/API.html#delete)") + err.status = 400 + throw err } - const body = JSON.stringify(req.body) + + const deleteBody = JSON.stringify(req.body) const deleteOptions = { - body, + body: deleteBody, method: 'DELETE', headers: { 'user-agent': 'Tiny-Things/1.0', + 'Origin': process.env.ORIGIN, 'Authorization': `Bearer ${process.env.ACCESS_TOKEN}`, 'Content-Type' : "application/json; charset=utf-8" } } const deleteURL = `${process.env.RERUM_API_ADDR}delete` - let errored = false - const result = await fetch(deleteURL, deleteOptions).then(async res=>{ - if (!res.ok) { - errored = true - return res + await fetchRerum(deleteURL, deleteOptions) + .then(async (resp) => { + if (resp.ok) return + let rerumErrorMessage + try { + rerumErrorMessage = `${resp.status ?? 500}: ${deleteURL} - ${await resp.text()}` + } catch (e) { + rerumErrorMessage = `500: ${deleteURL} - A RERUM error occurred` } - return res.text() - }) - .catch(err => { - throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) + const err = new Error(rerumErrorMessage) + err.status = 502 + throw err }) - // Send RERUM error responses to error-messenger.js - if (errored) return next(result) - res.sendStatus(204) + res.status(204).end() } catch (err) { - next(err) + console.error(err) + res.status(err.status ?? 500).type('text/plain').send(err.message ?? 'An error occurred') } }) -/* DELETE a delete to the thing. */ -router.delete('/:id', async (req, res, next) => { +/* DELETE an object by ID via the RERUM API. */ +router.delete('/:id', checkAccessToken, async (req, res, next) => { try { const deleteURL = `${process.env.RERUM_API_ADDR}delete/${req.params.id}` const deleteOptions = { - method: 'DELETE', + method: "DELETE", headers: { 'user-agent': 'Tiny-Things/1.0', + 'Origin': process.env.ORIGIN, 'Authorization': `Bearer ${process.env.ACCESS_TOKEN}`, } } - let errored = false - const result = await fetch(deleteURL, deleteOptions).then(res => { - if (!res.ok) errored = true - return res - }) - .catch(err => { - throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) + await fetchRerum(deleteURL, deleteOptions) + .then(async (resp) => { + if (resp.ok) return + // The response from RERUM indicates a failure, likely with a specific code and textual body + let rerumErrorMessage + try { + rerumErrorMessage = `${resp.status ?? 500}: ${deleteURL} - ${await resp.text()}` + } catch (e) { + rerumErrorMessage = `500: ${deleteURL} - A RERUM error occurred` + } + const err = new Error(rerumErrorMessage) + err.status = 502 + throw err }) - // Send RERUM error responses to error-messenger.js - if (errored) return next(result) - res.sendStatus(204) + res.status(204).end() } catch (err) { - next(err) + console.error(err) + res.status(err.status ?? 500).type('text/plain').send(err.message ?? 'An error occurred') } }) diff --git a/routes/overwrite.js b/routes/overwrite.js index c138557..696ed02 100644 --- a/routes/overwrite.js +++ b/routes/overwrite.js @@ -1,6 +1,7 @@ import express from "express" import checkAccessToken from "../tokens.js" -import { httpError, verifyJsonContentType } from "../rest.js" +import { verifyJsonContentType } from "../rest.js" +import { createRerumNetworkError, fetchRerum } from "../rerum.js" const router = express.Router() /* PUT an overwrite to the thing. */ @@ -20,54 +21,60 @@ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) body: JSON.stringify(overwriteBody), headers: { 'user-agent': 'Tiny-Things/1.0', + 'Origin': process.env.ORIGIN, 'Authorization': `Bearer ${process.env.ACCESS_TOKEN}`, 'Content-Type' : "application/json;charset=utf-8" } } // Pass through If-Overwritten-Version header if present - const ifOverwrittenVersion = Object.hasOwn(req.headers, 'if-overwritten-version') ? req.headers['if-overwritten-version'] : null - if (ifOverwrittenVersion !== null) { + const ifOverwrittenVersion = req.headers['if-overwritten-version'] + if (ifOverwrittenVersion) { overwriteOptions.headers['If-Overwritten-Version'] = ifOverwrittenVersion } // Check for __rerum.isOverwritten in body and use as If-Overwritten-Version header - const isOverwrittenValue = Object.hasOwn(req.body?.__rerum ?? {}, "isOverwritten") ? req.body.__rerum.isOverwritten : null - if (isOverwrittenValue !== null) { + const isOverwrittenValue = req.body?.__rerum?.isOverwritten + if (isOverwrittenValue) { overwriteOptions.headers['If-Overwritten-Version'] = isOverwrittenValue } const overwriteURL = `${process.env.RERUM_API_ADDR}overwrite` - let errored = false - const response = await fetch(overwriteURL, overwriteOptions) - .then(async rerum_res=>{ - if (rerum_res.ok) return rerum_res.json() - errored = true - const contentType = rerum_res.headers.get("Content-Type")?.toLowerCase() ?? "" - if (contentType.includes("json")) { - // Special handling. This does not go through to error-messenger.js - if (rerum_res.status === 409) { - const currentVersion = await rerum_res.json() - return res.status(409).json(currentVersion) - } + const rerumResponse = await fetchRerum(overwriteURL, overwriteOptions) + .then(async (resp) => { + if (resp.ok) return resp.json() + // Handle 409 conflict error for version mismatch (optimistic locking) + if (resp.status === 409) { + const conflictBody = await resp.json() + const err = new Error("Version conflict") + err.status = 409 + err.body = conflictBody + throw err } - return rerum_res - }) - .catch(err => { - throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) + // The response from RERUM indicates a failure, likely with a specific code and textual body + let rerumErrorMessage + try { + rerumErrorMessage = `${resp.status ?? 500}: ${overwriteURL} - ${await resp.text()}` + } catch (e) { + rerumErrorMessage = `500: ${overwriteURL} - A RERUM error occurred` + } + const err = new Error(rerumErrorMessage) + err.status = 502 + throw err }) - // Send RERUM error responses to error-messenger.js - if (errored) return next(response) - const result = response - const location = result?.["@id"] ?? result?.id - const responseBody = { ...req.body, ...(result ?? {}) } - if (location) { - res.setHeader("Location", location) + if (!(rerumResponse.id || rerumResponse["@id"])) { + // A 200 with garbled data, call it a fail + throw createRerumNetworkError(overwriteURL) } - res.status(200).json(responseBody) + res.setHeader("Location", rerumResponse["@id"] ?? rerumResponse.id) + res.status(200).json(rerumResponse) } catch (err) { - next(err) + console.error(err) + if (err.status === 409) { + return res.status(409).json(err.body) + } + res.status(err.status ?? 500).type('text/plain').send(err.message ?? 'An error occurred') } }) diff --git a/routes/query.js b/routes/query.js index a2a69e5..a963556 100644 --- a/routes/query.js +++ b/routes/query.js @@ -1,54 +1,62 @@ import express from "express" import { verifyJsonContentType } from "../rest.js" +import { fetchRerum } from "../rerum.js" const router = express.Router() /* POST a query to the thing. */ router.post('/', verifyJsonContentType, async (req, res, next) => { const lim = req.query.limit ?? 10 const skip = req.query.skip ?? 0 - const limit = Number.parseInt(lim, 10) - const offset = Number.parseInt(skip, 10) try { - if (!req.body || typeof req.body !== "object" || Array.isArray(req.body)) { - res.status(400).send("Query payload must be a non-empty JSON object.") - return - } - if (Object.keys(req.body).length === 0) { - res.status(400).send("Query payload must not be an empty object.") - return + // check body for JSON + const queryBody = JSON.stringify(req.body) + // If there is an empty query with [] or {}, we consider that a query for all data, + // which we don't want to allow. We will throw a 400 error. + if (queryBody === '{}' || queryBody === '[]') { + const err = new Error("Empty query is not allowed. Please provide a valid query in the request body.") + err.status = 400 + throw err } - if (!Number.isInteger(limit) || !Number.isInteger(offset) || limit < 0 || offset < 0) { - res.status(400).send("`limit` and `skip` values must be non-negative integers or omitted.") - return + // check limit and skip for INT + if (Number.isNaN(Number.parseInt(lim, 10) + Number.parseInt(skip, 10)) + || (lim < 0) + || (skip < 0)) { + const err = new Error("`limit` and `skip` values must be positive integers or omitted.") + err.status = 400 + throw err } - const body = JSON.stringify(req.body) + const queryOptions = { method: 'POST', - body, + body: queryBody, headers: { 'user-agent': 'Tiny-Things/1.0', - 'Authorization': `Bearer ${process.env.RERUM_TOKEN}`, // not required for query + 'Origin': process.env.ORIGIN, + 'Authorization': `Bearer ${process.env.ACCESS_TOKEN}`, // not required for query 'Content-Type' : "application/json;charset=utf-8" } } - const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${limit}&skip=${offset}` - let errored = false - const results = await fetch(queryURL, queryOptions).then(res=>{ - if (res.ok) return res.json() - errored = true - return res - }) - .catch(err => { + const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${lim}&skip=${skip}` + const rerumResponse = await fetchRerum(queryURL, queryOptions) + .then(async (resp) => { + if (resp.ok) return resp.json() + // The response from RERUM indicates a failure, likely with a specific code and textual body + let rerumErrorMessage + try { + rerumErrorMessage = `${resp.status ?? 500}: ${queryURL} - ${await resp.text()}` + } catch (e) { + rerumErrorMessage = `500: ${queryURL} - A RERUM error occurred` + } + const err = new Error(rerumErrorMessage) + err.status = 502 throw err }) - // Send RERUM error responses to error-messenger.js - if (errored) return next(results) - res.status(200) - res.send(results) + res.status(200).json(rerumResponse) } - catch (err) { // a dumb catch-all for Tiny Stuff - next(err) + catch (err) { + console.error(err) + res.status(err.status ?? 500).type('text/plain').send(err.message ?? 'An error occurred') } }) diff --git a/routes/update.js b/routes/update.js index 5956f68..d827382 100644 --- a/routes/update.js +++ b/routes/update.js @@ -1,46 +1,57 @@ import express from "express" import checkAccessToken from "../tokens.js" -import { httpError, verifyJsonContentType } from "../rest.js" +import { verifyJsonContentType } from "../rest.js" +import { createRerumNetworkError, fetchRerum } from "../rerum.js" const router = express.Router() /* PUT an update to the thing. */ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) => { try { - // check for @id in body. Any value is valid. Lack of value is a bad request. - if (!req?.body || !(req.body['@id'] ?? req.body.id)) { - res.status(400).send("No record id to update! (https://store.rerum.io/v1/API.html#update)") - return + // check for @id; any value is valid + if (!(req.body['@id'] ?? req.body.id)) { + const err = new Error("No record id to update! (https://store.rerum.io/API.html#update)") + err.status = 400 + throw err } - // check body for JSON - const body = JSON.stringify(req.body) + + const updateBody = JSON.stringify(req.body) const updateOptions = { method: 'PUT', - body, + body: updateBody, headers: { 'user-agent': 'Tiny-Things/1.0', + 'Origin': process.env.ORIGIN, 'Authorization': `Bearer ${process.env.ACCESS_TOKEN}`, 'Content-Type' : "application/json;charset=utf-8" } } const updateURL = `${process.env.RERUM_API_ADDR}update` - let errored = false - const result = await fetch(updateURL, updateOptions).then(res=>{ - if (res.ok) return res.json() - errored = true - return res - }) - .catch(err => { - throw httpError(err.message || "TinyNode could not communicate with RERUM.", 502) + const rerumResponse = await fetchRerum(updateURL, updateOptions) + .then(async (resp) => { + if (resp.ok) return resp.json() + // The response from RERUM indicates a failure, likely with a specific code and textual body + let rerumErrorMessage + try { + rerumErrorMessage = `${resp.status ?? 500}: ${updateURL} - ${await resp.text()}` + } catch (e) { + rerumErrorMessage = `500: ${updateURL} - A RERUM error occurred` + } + const err = new Error(rerumErrorMessage) + err.status = 502 + throw err }) - // Send RERUM error responses to error-messenger.js - if (errored) return next(result) - res.setHeader("Location", result["@id"] ?? result.id) - res.status(200) - res.json(result) + if (!(rerumResponse.id || rerumResponse["@id"])) { + // A 200 with garbled data, call it a fail + throw createRerumNetworkError(updateURL) + } + + res.setHeader("Location", rerumResponse["@id"] ?? rerumResponse.id) + res.status(200).json(rerumResponse) } catch (err) { - next(err) + console.error(err) + res.status(err.status ?? 500).type('text/plain').send(err.message ?? 'An error occurred') } }) From a85edbca5a611446ea305bc489c1f074ccf065e9 Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Thu, 2 Apr 2026 16:29:02 -0500 Subject: [PATCH 3/6] Improve error handling and validation Add explicit error body handling and streamline error propagation. error-messenger now detects ReadableStream bodies and uses an explicitBody field (fixing a typo in the default message). rest.httpError sets error.errorBody when a body is provided so original bodies are preserved. Routes enforce request body presence before checking @id and now throw a 400 error instead of directly sending a response (overwrite.js, update.js). query.js clarifies the limit/skip validation message to require non-negative integers. Tests updated to reflect changed status codes/messages and the query tests now mount the messenger middleware. --- error-messenger.js | 13 +++++++++++-- rest.js | 1 + routes/__tests__/create.test.js | 6 +++--- routes/__tests__/query.test.js | 2 ++ routes/overwrite.js | 7 ++++--- routes/query.js | 2 +- routes/update.js | 2 +- 7 files changed, 23 insertions(+), 10 deletions(-) diff --git a/error-messenger.js b/error-messenger.js index 94d12cb..3260950 100644 --- a/error-messenger.js +++ b/error-messenger.js @@ -1,6 +1,7 @@ /** * Errors from RERUM are a response code with a text body (except those handled specifically upstream) * We want to send the same error code and message through. It is assumed to be RESTful and useful. + * This is a fallback for unhandled errors from RERUM, and should not be used for expected errors that are handled upstream (e.g. 409 conflict for version mismatch on overwrite). It will also handle network errors that occur when trying to reach RERUM, which will be mapped to a 502 Bad Gateway with a message indicating a RERUM network error. * This will also handle generic (500) app level errors, as well as app level 404 errors. * * @param rerum_error_res A Fetch API Response object from a fetch() to RERUM that encountered an error. Explanatory text is in .text(). In some cases it is a unhandled generic (500) app level Error. @@ -13,10 +14,18 @@ export async function messenger(rerum_error_res, req, res, next) { return } + const hasReadableStream = typeof globalThis.ReadableStream === "function" + const explicitBody = rerum_error_res.errorBody ?? ( + rerum_error_res.body !== undefined + && (!hasReadableStream || !(rerum_error_res.body instanceof globalThis.ReadableStream)) + ? rerum_error_res.body + : undefined + ) + const error = { - message: rerum_error_res.statusMessage ?? rerum_error_res.message ?? "A server error has occured", + message: rerum_error_res.statusMessage ?? rerum_error_res.message ?? "A server error has occurred", status: rerum_error_res.statusCode ?? rerum_error_res.status ?? 500, - body: rerum_error_res.body + body: explicitBody } if (error.body !== undefined) { diff --git a/rest.js b/rest.js index 23c45d3..6361214 100644 --- a/rest.js +++ b/rest.js @@ -7,6 +7,7 @@ export function httpError(message, status = 500, body) { const error = new Error(message) error.status = status if (body !== undefined) { + error.errorBody = body error.body = body } return error diff --git a/routes/__tests__/create.test.js b/routes/__tests__/create.test.js index b9f47cc..3952880 100644 --- a/routes/__tests__/create.test.js +++ b/routes/__tests__/create.test.js @@ -129,8 +129,8 @@ describe("Check that TinyNode create route propagates upstream and network error .send({ "test": "item" }) .then(resp => resp) .catch(err => err) - expect(response.statusCode).toBe(503) - expect(response.text).toBe("Upstream create failure") + expect(response.statusCode).toBe(502) + expect(response.text).toContain("Upstream create failure") global.fetch = jest.fn(() => Promise.reject(new Error("socket hang up"))) @@ -141,7 +141,7 @@ describe("Check that TinyNode create route propagates upstream and network error .then(resp => resp) .catch(err => err) expect(response.statusCode).toBe(502) - expect(response.text).toContain("socket hang up") + expect(response.text).toContain("A RERUM error occurred") }) }) diff --git a/routes/__tests__/query.test.js b/routes/__tests__/query.test.js index bc8bac0..f116bb4 100644 --- a/routes/__tests__/query.test.js +++ b/routes/__tests__/query.test.js @@ -2,6 +2,7 @@ import express from "express" import request from "supertest" import { jest } from "@jest/globals" import queryRoute from "../query.js" +import { messenger } from "../../error-messenger.js" //import app from "../../app.js" const routeTester = new express() @@ -9,6 +10,7 @@ routeTester.use(express.json()) routeTester.use(express.urlencoded({ extended: false })) routeTester.use("/query", queryRoute) routeTester.use("/app/query", queryRoute) +routeTester.use(messenger) const rerum_uri = `${process.env.RERUM_ID_PATTERN}_not_` diff --git a/routes/overwrite.js b/routes/overwrite.js index 696ed02..0b8d47e 100644 --- a/routes/overwrite.js +++ b/routes/overwrite.js @@ -11,9 +11,10 @@ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) const overwriteBody = req.body // check for @id; any value is valid - if (!(overwriteBody['@id'] ?? overwriteBody.id)) { - res.status(400).send("No record id to overwrite! (https://store.rerum.io/API.html#overwrite)") - return + if (!overwriteBody || !(overwriteBody['@id'] ?? overwriteBody.id)) { + const err = new Error("No record id to overwrite! (https://store.rerum.io/API.html#overwrite)") + err.status = 400 + throw err } const overwriteOptions = { diff --git a/routes/query.js b/routes/query.js index a963556..cbb06e4 100644 --- a/routes/query.js +++ b/routes/query.js @@ -22,7 +22,7 @@ router.post('/', verifyJsonContentType, async (req, res, next) => { if (Number.isNaN(Number.parseInt(lim, 10) + Number.parseInt(skip, 10)) || (lim < 0) || (skip < 0)) { - const err = new Error("`limit` and `skip` values must be positive integers or omitted.") + const err = new Error("`limit` and `skip` values must be non-negative integers or omitted.") err.status = 400 throw err } diff --git a/routes/update.js b/routes/update.js index d827382..cbb7d26 100644 --- a/routes/update.js +++ b/routes/update.js @@ -9,7 +9,7 @@ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) try { // check for @id; any value is valid - if (!(req.body['@id'] ?? req.body.id)) { + if (!req.body || !(req.body['@id'] ?? req.body.id)) { const err = new Error("No record id to update! (https://store.rerum.io/API.html#update)") err.status = 400 throw err From 0640e64e327fd87b701096457b341918991e9b26 Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Thu, 2 Apr 2026 16:46:42 -0500 Subject: [PATCH 4/6] Use httpError helper and payload for errors Standardize error handling by introducing/using httpError and a `payload` property for error bodies. rest.js now sets error.payload instead of errorBody/body. error-messenger.js was simplified to send rerum_error_res.payload and dropped the ReadableStream/body detection logic. Routes (overwrite, query, update) now import and throw httpError for validation and upstream RERUM errors instead of creating Error objects and manually setting status codes. This centralizes HTTP error construction and response payload handling. --- error-messenger.js | 13 ++----------- rest.js | 3 +-- routes/overwrite.js | 6 ++---- routes/query.js | 14 ++++---------- routes/update.js | 10 +++------- 5 files changed, 12 insertions(+), 34 deletions(-) diff --git a/error-messenger.js b/error-messenger.js index 3260950..db4d536 100644 --- a/error-messenger.js +++ b/error-messenger.js @@ -14,23 +14,14 @@ export async function messenger(rerum_error_res, req, res, next) { return } - const hasReadableStream = typeof globalThis.ReadableStream === "function" - const explicitBody = rerum_error_res.errorBody ?? ( - rerum_error_res.body !== undefined - && (!hasReadableStream || !(rerum_error_res.body instanceof globalThis.ReadableStream)) - ? rerum_error_res.body - : undefined - ) - const error = { message: rerum_error_res.statusMessage ?? rerum_error_res.message ?? "A server error has occurred", status: rerum_error_res.statusCode ?? rerum_error_res.status ?? 500, - body: explicitBody } - if (error.body !== undefined) { + if (rerum_error_res.payload !== undefined) { console.error(error) - res.status(error.status).json(error.body) + res.status(error.status).json(rerum_error_res.payload) return } diff --git a/rest.js b/rest.js index 6361214..328cbb6 100644 --- a/rest.js +++ b/rest.js @@ -7,8 +7,7 @@ export function httpError(message, status = 500, body) { const error = new Error(message) error.status = status if (body !== undefined) { - error.errorBody = body - error.body = body + error.payload = body } return error } diff --git a/routes/overwrite.js b/routes/overwrite.js index 0b8d47e..7105cbf 100644 --- a/routes/overwrite.js +++ b/routes/overwrite.js @@ -1,6 +1,6 @@ import express from "express" import checkAccessToken from "../tokens.js" -import { verifyJsonContentType } from "../rest.js" +import { httpError, verifyJsonContentType } from "../rest.js" import { createRerumNetworkError, fetchRerum } from "../rerum.js" const router = express.Router() @@ -12,9 +12,7 @@ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) const overwriteBody = req.body // check for @id; any value is valid if (!overwriteBody || !(overwriteBody['@id'] ?? overwriteBody.id)) { - const err = new Error("No record id to overwrite! (https://store.rerum.io/API.html#overwrite)") - err.status = 400 - throw err + throw httpError("No record id to overwrite! (https://store.rerum.io/API.html#overwrite)", 400) } const overwriteOptions = { diff --git a/routes/query.js b/routes/query.js index cbb06e4..9f84fbf 100644 --- a/routes/query.js +++ b/routes/query.js @@ -1,5 +1,5 @@ import express from "express" -import { verifyJsonContentType } from "../rest.js" +import { httpError, verifyJsonContentType } from "../rest.js" import { fetchRerum } from "../rerum.js" const router = express.Router() @@ -14,17 +14,13 @@ router.post('/', verifyJsonContentType, async (req, res, next) => { // If there is an empty query with [] or {}, we consider that a query for all data, // which we don't want to allow. We will throw a 400 error. if (queryBody === '{}' || queryBody === '[]') { - const err = new Error("Empty query is not allowed. Please provide a valid query in the request body.") - err.status = 400 - throw err + throw httpError("Empty query is not allowed. Please provide a valid query in the request body.", 400) } // check limit and skip for INT if (Number.isNaN(Number.parseInt(lim, 10) + Number.parseInt(skip, 10)) || (lim < 0) || (skip < 0)) { - const err = new Error("`limit` and `skip` values must be non-negative integers or omitted.") - err.status = 400 - throw err + throw httpError("`limit` and `skip` values must be non-negative integers or omitted.", 400) } const queryOptions = { @@ -48,9 +44,7 @@ router.post('/', verifyJsonContentType, async (req, res, next) => { } catch (e) { rerumErrorMessage = `500: ${queryURL} - A RERUM error occurred` } - const err = new Error(rerumErrorMessage) - err.status = 502 - throw err + throw httpError(rerumErrorMessage, 502) }) res.status(200).json(rerumResponse) } diff --git a/routes/update.js b/routes/update.js index cbb7d26..5e90755 100644 --- a/routes/update.js +++ b/routes/update.js @@ -1,6 +1,6 @@ import express from "express" import checkAccessToken from "../tokens.js" -import { verifyJsonContentType } from "../rest.js" +import { httpError, verifyJsonContentType } from "../rest.js" import { createRerumNetworkError, fetchRerum } from "../rerum.js" const router = express.Router() @@ -10,9 +10,7 @@ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) try { // check for @id; any value is valid if (!req.body || !(req.body['@id'] ?? req.body.id)) { - const err = new Error("No record id to update! (https://store.rerum.io/API.html#update)") - err.status = 400 - throw err + throw httpError("No record id to update! (https://store.rerum.io/API.html#update)", 400) } const updateBody = JSON.stringify(req.body) @@ -37,9 +35,7 @@ router.put('/', verifyJsonContentType, checkAccessToken, async (req, res, next) } catch (e) { rerumErrorMessage = `500: ${updateURL} - A RERUM error occurred` } - const err = new Error(rerumErrorMessage) - err.status = 502 - throw err + throw httpError(rerumErrorMessage, 502) }) if (!(rerumResponse.id || rerumResponse["@id"])) { // A 200 with garbled data, call it a fail From 81945a37a2b71b0042c37bad1cfa9828589cf9b3 Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Thu, 2 Apr 2026 16:49:33 -0500 Subject: [PATCH 5/6] Rename httpError parameter to payload Change httpError signature from (message, status, body) to (message, status, payload) and set error.payload = payload. Note: the conditional still checks 'body !== undefined', which appears to be an inconsistent leftover and may prevent payload from being attached; update the check to 'payload !== undefined' as a follow-up. --- rest.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest.js b/rest.js index 328cbb6..585720c 100644 --- a/rest.js +++ b/rest.js @@ -3,11 +3,11 @@ const acceptedJsonContentTypes = new Set([ "application/ld+json" ]) -export function httpError(message, status = 500, body) { +export function httpError(message, status = 500, payload) { const error = new Error(message) error.status = status if (body !== undefined) { - error.payload = body + error.payload = payload } return error } From 7cca3f068ed604eb0414df79c41e06e25fdc6561 Mon Sep 17 00:00:00 2001 From: cubap Date: Mon, 13 Apr 2026 13:25:57 -0500 Subject: [PATCH 6/6] fixes all tests --- rest.js | 2 +- routes/__tests__/mount.test.js | 33 +++++++++++++++++++++------------ tokens.js | 14 +++++++++++++- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/rest.js b/rest.js index 585720c..266495c 100644 --- a/rest.js +++ b/rest.js @@ -6,7 +6,7 @@ const acceptedJsonContentTypes = new Set([ export function httpError(message, status = 500, payload) { const error = new Error(message) error.status = status - if (body !== undefined) { + if (payload !== undefined) { error.payload = payload } return error diff --git a/routes/__tests__/mount.test.js b/routes/__tests__/mount.test.js index ff2dd00..002a0dc 100644 --- a/routes/__tests__/mount.test.js +++ b/routes/__tests__/mount.test.js @@ -32,23 +32,32 @@ afterEach(() => { * @returns {boolean} - True if all routes are found */ function routeExists(routes) { - const stack = app.router.stack const foundRoutes = new Set() - for (const layer of stack) { - if (layer.matchers && layer.matchers[0]) { - const matcher = layer.matchers[0] - // Test each route against this layer's matcher - for (const route of routes) { - if (matcher(route)) foundRoutes.add(route) + const scanStack = (stack = []) => { + for (const layer of stack) { + if (layer.matchers && layer.matchers[0]) { + const matcher = layer.matchers[0] + for (const route of routes) { + if (matcher(route)) foundRoutes.add(route) + } } - } - // Also check route.path directly if it exists - if (layer.route && layer.route.path) { - for (const route of routes) { - if (layer.route.path === route) foundRoutes.add(route) + if (layer.regexp) { + for (const route of routes) { + if (layer.regexp.test(route)) foundRoutes.add(route) + } + } + if (layer.route && layer.route.path) { + for (const route of routes) { + if (layer.route.path === route) foundRoutes.add(route) + } + } + if (layer.handle?.stack) { + scanStack(layer.handle.stack) } } } + + scanStack(app._router?.stack) // Check if all expected routes were found return routes.every(route => foundRoutes.has(route)) } diff --git a/tokens.js b/tokens.js index 018a8b0..3f50a61 100644 --- a/tokens.js +++ b/tokens.js @@ -6,7 +6,19 @@ import { parse, stringify } from "envfile" const sourcePath = '.env' // https://stackoverflow.com/a/69058154/1413302 -const isTokenExpired = (token) => (Date.now() >= JSON.parse(Buffer.from(token.split('.')[1], 'base64').toString()).exp * 1000) +const isTokenExpired = (token) => { + try { + const payload = token?.split('.')?.[1] + if (!payload) return false + const exp = JSON.parse(Buffer.from(payload, 'base64').toString())?.exp + if (!Number.isFinite(exp)) return false + return Date.now() >= exp * 1000 + } + catch (err) { + // Treat malformed tokens as non-expired so middleware does not block requests. + return false + } +} /** * Use the privately stored refresh token to generate a new access token for