CenterForDigitalHumanities / CenterForDigitalHumanities/TinyNode
Misleading 5xx errors: `application/ld+json` bodies are never parsed, and bodied `DELETE /delete` 404s upstream
- Dominant language
- JavaScript
- Stars
- 1
- Forks
- 3
- Avg merge
- 29m
- Merged PRs (30d)
- 1
Description
## Summary
While smoke testing the package updates on the `8-24-26-packages` branch, two separate 502-producing
defects turned up. Neither is caused by the dependency bumps — both reproduce on `main` as well. They
are filed together because both were found the same way, but they are independent problems.
The first one is the more serious of the two: every route mishandles `application/ld+json`, and
`/create` leaks a raw `TypeError` to the client as a 500.
## Finding 1: `application/ld+json` passes the content-type gate but is never parsed
`rest.js` accepts `application/ld+json` as a valid request content type
([rest.js#L1-L4](https://github.com/CenterForDigitalHumanities/TinyNode/blob/main/rest.js#L1-L4)), so `verifyJsonContentType` lets the request through
instead of returning 415. But `app.js` only mounts `express.json()`
([app.js#L20](https://github.com/CenterForDigitalHumanities/TinyNode/blob/main/app.js#L20)), whose body-parser `type` option defaults to
`application/json` and does **not** match the `+json` structured suffix. `express.text()` on the next
line only claims `text/plain`.
The result is that no parser claims the body, so `req.body` is `undefined` (Express 5 no longer
defaults it to `{}`), and every route then misbehaves in its own way.
### Reproduction
Every request below sends a well-formed JSON body with a valid `@id`, and differs from a working
request *only* in the `Content-Type` header:
```bash
curl -X POST -H 'Content-Type: application/ld+json' \
--data '{"@id":"https://devstore.rerum.io/v1/id/000000000000000000000000","type":"Probe"}' \
http://localhost:3002/create
```
### Observed behavior
| Route | Status | Response body |
|---|---|---|
| `POST /query` | 502 | `400: .../query?limit=10&skip=0 - Detected empty JSON object.` |
| `POST /create` | 500 | `Cannot read properties of undefined (reading 'id')` |
| `PUT /update` | 400 | `No record id to update!` |
| `PUT /overwrite` | 400 | `No record id to overwrite!` |
| `DELETE /delete` | 400 | `No record id to delete!` |
The identical body sent as `application/json` returns 200/201 as expected.
### Why each is wrong
- **`/create` returns 500 and leaks an internal error.** `req.body.id` on
[routes/create.js#L12](https://github.com/CenterForDigitalHumanities/TinyNode/blob/main/routes/create.js#L12) throws a `TypeError` against `undefined`. The route's
`catch` has no `err.status`, so it falls through to 500 and sends the raw V8 message
(`Cannot read properties of undefined (reading 'id')`) to the client. That is an internal
implementation detail on the wire, and it contradicts our own guidance about returning generic
error messages rather than internal details.
- **`/query` returns 502.** `JSON.stringify(undefined)` is `undefined`, not `'{}'`, so the empty-query
guard on [routes/query.js#L16](https://github.com/CenterForDigitalHumanities/TinyNode/blob/main/routes/query.js#L16) is bypassed. The request is forwarded upstream
with no body, RERUM 400s, and TinyNode maps that to 502 — reporting an upstream failure for what is
really a client error we should have caught.
- **`/update`, `/overwrite`, `/delete` return a misleading 400.** The status code is defensible, but
the message tells the caller they omitted a record id when they plainly supplied one. That will send
people debugging the wrong thing.
### Suggested fix
Either accept ld+json properly or reject it, but stop doing both. Accepting it is the smaller change
and matches what `rest.js` already advertises:
```js
app.use(express.json({ type: ["application/json", "application/ld+json"] }))
```
Given this is a JSON-LD store, accepting it seems clearly correct. Worth deciding separately whether
`/create` should also guard `req.body` defensively so a missing body can never produce a raw
`TypeError` again — the same class of bug would resurface for any future unparsed content type.
This is a gap left by #110, which introduced `rest.js` and the ld+json allowance without a matching
parser change.
## Finding 2: bodied `DELETE /delete` returns 502 because the upstream endpoint does not exist
The legacy bodied-delete handler ([routes/delete.js#L27](https://github.com/CenterForDigitalHumanities/TinyNode/blob/main/routes/delete.js#L27)) posts to
`${RERUM_API_ADDR}delete`. That endpoint does not exist on devstore — it 404s, and TinyNode maps the
404 to a 502.
### Reproduction
```bash
# create something to delete, then:
curl -X DELETE -H 'Content-Type: application/json' \
--data '{"@id":"https://devstore.rerum.io/v1/id/"}' \
http://localhost:3002/delete
# -> HTTP 502
# 404: https://devstore.rerum.io/v1/api/delete - This page does not exist
```
Confirmed against RERUM directly, so this is upstream and not a TinyNode routing mistake:
```bash
curl -i -X DELETE -H 'Content-Type: application/json' \
--data '{"@id":"..."}' https://devstore.rerum.io/v1/api/delete
# -> HTTP 404
```
The path form works correctly:
```bash
curl -X DELETE http://localhost:3002/delete/
# -> HTTP 204
```
### What to decide
This route cannot succeed against devstore in its current form, so one of the following is needed:
- Confirm whether `/v1/api/delete` with a body is supposed to exist on RERUM. If it is, this is a
RERUM bug and TinyNode is fine.
- If it is not coming back, have the bodied handler extract the id from `@id` and delegate to the
path form, so the documented legacy call keeps working.
- Failing both, deprecate the bodied form and return a 400 that says so, rather than a 502 that
implies an upstream outage.
Related to #89, which covers different odd behavior on the same endpoint (a delete with no id
returning 204). That specific case now correctly returns 400, so #89 may be partly stale.
## Environment
- Branch `8-24-26-packages`, also reproduces on `main`
- Node v24.19.0, npm 12.0.2
- `RERUM_API_ADDR=https://devstore.rerum.io/v1/api/`
- All 80 route tests and 5 e2e tests pass, so neither defect is currently covered by a test
Contributor guide
Research direction
Start with app.js and rest.js to trace the content-type and body-parser behavior, then inspect routes/create.js, routes/query.js, and routes/delete.js. Run the existing route tests and reproduce both cases with the supplied curl commands. Done means the ld+json behavior is consistent and tested, while the bodied DELETE contract is confirmed and no longer reports a misleading upstream failure.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- express, javascript, nodejs
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100