diff --git a/UPSTREAM_CANDIDATES.md b/UPSTREAM_CANDIDATES.md index cc5141b..095ec9d 100644 --- a/UPSTREAM_CANDIDATES.md +++ b/UPSTREAM_CANDIDATES.md @@ -15,7 +15,7 @@ This document tracks candidate updates identified from TinyPen that may be upstr | 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 | investigate-first | PR #39 and related | `routes/*.js`, `error-messenger.js` | Normalize error objects/status handling before broad refactor. | +| 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. | @@ -42,3 +42,13 @@ This document tracks candidate updates identified from TinyPen that may be upstr 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 43ce282..db4d536 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. @@ -9,26 +10,40 @@ * @param next The Express next() operator, unused here but required to support the middleware chain. */ export async function messenger(rerum_error_res, req, res, next) { - if (res.headersSent) { - return - } - let error = {} - let rerum_err_text - 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() - } - catch (err) { - // It is some 500 - rerum_err_text = undefined + if (res.headersSent) { + return + } + + 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, + } + + if (rerum_error_res.payload !== undefined) { + console.error(error) + res.status(error.status).json(rerum_error_res.payload) + return + } + + try { + 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 } - 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 occurred` + + const rerumErrText = await rerum_error_res.text() + if (rerumErrText) { + error.message = rerumErrText } - 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) + } + catch (err) { + // Fall back to the status/message values already collected above. + } + + console.error(error) + res.set("Content-Type", "text/plain; charset=utf-8") + res.status(error.status).send(error.message) } 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/rest.js b/rest.js index a6ae2af..266495c 100644 --- a/rest.js +++ b/rest.js @@ -3,6 +3,15 @@ const acceptedJsonContentTypes = new Set([ "application/ld+json" ]) +export function httpError(message, status = 500, payload) { + const error = new Error(message) + error.status = status + if (payload !== undefined) { + error.payload = payload + } + return error +} + function getContentType(req) { const rawContentType = req.headers?.["content-type"] if (!rawContentType) { diff --git a/routes/__tests__/create.test.js b/routes/__tests__/create.test.js index 9dae550..3952880 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(502) + expect(response.text).toContain("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("A RERUM error occurred") + }) +}) + /** * Full integration test. Checks the TinyNode app create endpoint functionality and RERUM connection. * 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__/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/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/create.js b/routes/create.js index 535aec7..4bc03cc 100644 --- a/routes/create.js +++ b/routes/create.js @@ -1,44 +1,55 @@ import express from "express" import checkAccessToken from "../tokens.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 => { + 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 d70fab7..4bc3912 100644 --- a/routes/delete.js +++ b/routes/delete.js @@ -1,72 +1,83 @@ import express from "express" import checkAccessToken from "../tokens.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(res=>{ - if (!res.ok) errored = true - return res.text() - }) - .catch(err => { + 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` + } + 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 => { + 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 107e93d..7105cbf 100644 --- a/routes/overwrite.js +++ b/routes/overwrite.js @@ -1,6 +1,7 @@ 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() /* PUT an overwrite to the thing. */ @@ -10,9 +11,8 @@ 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)) { + throw httpError("No record id to overwrite! (https://store.rerum.io/API.html#overwrite)", 400) } const overwriteOptions = { @@ -20,53 +20,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 - if ((rerum_res.headers?.get("Content-Type") ?? "").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 => { + // 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 c477e7d..9f84fbf 100644 --- a/routes/query.js +++ b/routes/query.js @@ -1,52 +1,56 @@ import express from "express" -import { verifyJsonContentType } from "../rest.js" +import { httpError, 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 limit = Number.parseInt(req.query.limit ?? 10, 10) - const skip = Number.parseInt(req.query.skip ?? 0, 10) + const lim = req.query.limit ?? 10 + const skip = req.query.skip ?? 0 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 + // 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 === '[]') { + throw httpError("Empty query is not allowed. Please provide a valid query in the request body.", 400) } - if (Object.keys(req.body).length === 0) { - res.status(400).send("Query payload must not be an empty object.") - return + // check limit and skip for INT + if (Number.isNaN(Number.parseInt(lim, 10) + Number.parseInt(skip, 10)) + || (lim < 0) + || (skip < 0)) { + throw httpError("`limit` and `skip` values must be non-negative integers or omitted.", 400) } - if (!Number.isInteger(limit) || !Number.isInteger(skip) || limit < 0 || skip < 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, + 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=${skip}` - let errored = false - const results = await fetch(queryURL, queryOptions).then(res=>{ - if (res.ok) return res.json() - errored = true - return res - }) - .catch(err => { - throw 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` + } + throw httpError(rerumErrorMessage, 502) }) - // 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 3a63d9d..5e90755 100644 --- a/routes/update.js +++ b/routes/update.js @@ -1,49 +1,53 @@ 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() /* 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 || !(req.body['@id'] ?? req.body.id)) { + throw httpError("No record id to update! (https://store.rerum.io/API.html#update)", 400) } - // 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 err + 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` + } + throw httpError(rerumErrorMessage, 502) }) - // 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(updateURL) } - res.status(200).json(responseBody) + + 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') } }) 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