CenterForDigitalHumanities / CenterForDigitalHumanities/TinyNode

Misleading 5xx errors: `application/ld+json` bodies are never parsed, and bodied `DELETE /delete` 404s upstream

Open
#129 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.