Skip to content

Make API endpoint validation more strict, improve errors#1585

Merged
cpeel merged 7 commits into
DistributedProofreaders:masterfrom
bpfoley:method-err
Jun 7, 2026
Merged

Make API endpoint validation more strict, improve errors#1585
cpeel merged 7 commits into
DistributedProofreaders:masterfrom
bpfoley:method-err

Conversation

@bpfoley

@bpfoley bpfoley commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Make API endpoint validation more strict and make the errors messages more helpful.

While here, improve some internal APIs:

  1. Add a ApiException::status_code readonly property to reduce boilerplate in derived exceptions
  2. Remove $query_params from all handler functions that don't have params.
  3. Add ApiException::fromException to reduce boilerplate when converting exceptions

Sandbox: https://www.pgdp.org/~cpeel/c.branch/method-err/

@bpfoley bpfoley force-pushed the method-err branch 6 times, most recently from 7eb81b5 to b1c660d Compare June 2, 2026 23:37
@bpfoley bpfoley marked this pull request as draft June 2, 2026 23:43
@bpfoley bpfoley changed the title Method err Make API endpoint validation more strict, improve errors Jun 2, 2026
@bpfoley bpfoley marked this pull request as ready for review June 3, 2026 14:10
Comment thread api/exceptions.inc
Comment thread api/ApiRouter.inc
Comment thread api/ApiRouter.inc Outdated
@cpeel cpeel requested review from 70ray, cpeel, jchaffraix and srjfoo June 4, 2026 18:10
@cpeel cpeel self-assigned this Jun 4, 2026
@cpeel

cpeel commented Jun 4, 2026

Copy link
Copy Markdown
Member

These are great improvements, Brian!

Before:

export API_KEY=review
curl -H "Accept: application/json" -H "X-API-KEY: $API_KEY" \
     -X GET "https://www.pgdp.org/api/v1/stats/site?blah=1"
{
    "server_time": "2026-06-04T18:15:37+00:00",
    "registered_users": 893,
    "active_users_1_day": 1,
    "active_users_7_day": 8,
    "active_users_30_day": 15
}

After:

export API_KEY=review
curl -H "Accept: application/json" -H "X-API-KEY: $API_KEY" \
     -X GET "https://www.pgdp.org/~cpeel/c.branch/method-err/api/v1/stats/site?blah=1"
{
    "error": "API endpoint /v1/stats/site takes no query parameters, but was called with blah",
    "code": 12
}

bpfoley added 7 commits June 4, 2026 20:09
Readonly properties were introduced in PHP 8.1
Handle the following cases:
1. A misspelled part is provided
2. A part for a node with no children
3. A missing part for nodes with children but no handler of its own.
This has two benefits:
1. It tightens up the type signature of the handler functions
   (as well as removing boilerplate).
2. It catches latent errors in the client.
Because of how the ouput JSON is built, api/v1/projects/ has the
effect of sliently deduplicating the field.

Report it as an error, as it may be a latent bug in the client
@bpfoley

bpfoley commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

Reproducers:

API_KEY=revoew
ROOT=https://www.pgdp.org/~bfoley/c.branch/method-err

# API endpoint v1/queues: Method WIBBLE not supported. Valid methods: GET
curl -i -X WIBBLE "$ROOT/api/v1/queues?roundid=P3&show=populated" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# 1. API endpoint /v1 has no part projectz. Valid child parts: documents, dictionaries, projects, queues, stats, storage.
curl -i -X GET "$ROOT/api/v1/projectz/projectID5e263d080c3f4/wordcheck/ai" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# 2. No children:
# API endpoint /v1/projects/projectID5e263d080c3f4/wordcheck has no part ai
curl -i -X GET "$ROOT/api/v1/projects/projectID5e263d080c3f4/wordcheck/ai" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# 3. Missing part.
# API endpoint /v1/stats is missing a part. Valid child parts: site, user.
curl -i -X GET "$ROOT/api/v1/stats" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# Invalid project state wibble. Valid states: ...
curl -i -X PUT "$ROOT/api/v1/projects/projectID5e263d080c3f4/checkout?state=wibble" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# Invalid page state wibble. Valid states: ...
curl -i -X PUT "$ROOT/api/v1/projects/projectID5e263d080c3f4/pages/042.png?state=F1.proj_avail&pagestate=wibble" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# API endpoint /v1/dictionaries takes no query parameters, but was called with param1, param2
curl -i -X GET "$ROOT/api/v1/dictionaries?param1=invalid&param2" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# Duplicate field[] args: title
curl -i -X GET "$ROOT/api/v1/projects?per_page=1&page=1&state=P3.proj_avail&field[]=title&field[]=title" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

# Check ApiException::fromException works using a deleted project
# try { ... } catch (NoProjectPageTable $exception) {
#     throw NotFoundError::fromException($exception);
# }
curl -i -X GET "$ROOT/api/v1/projects/projectID69668cd98b3b3/pages" \
    -H "Accept: application/json" \
    -H "X-API-KEY: $API_KEY"

@cpeel cpeel merged commit f49ea98 into DistributedProofreaders:master Jun 7, 2026
12 checks passed
@bpfoley bpfoley deleted the method-err branch June 8, 2026 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants