Skip to content

api: return helpful error when calling a function with too-old an API…#7833

Open
Abhishek01samal wants to merge 2 commits into
ether:developfrom
Abhishek01samal:fix-api-version-error
Open

api: return helpful error when calling a function with too-old an API…#7833
Abhishek01samal wants to merge 2 commits into
ether:developfrom
Abhishek01samal:fix-api-version-error

Conversation

@Abhishek01samal
Copy link
Copy Markdown

Summary

Improves the error message returned when an API function exists but is unavailable in the requested API version.

Previously, requests such as /api/1/copyPad returned a generic "no such function" error even though the function exists in newer API versions.

This change detects that case and returns a more helpful message indicating the minimum supported API version.

Fixes #6849

Testing

  • Added regression test for version mismatch handling
  • Verified both legacy and OpenAPI routing paths
  • Confirmed HTTP 404 with code 3 and expected error message

@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Return helpful error for API functions with too-old version

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Detects when API function exists but unavailable in requested version
• Returns helpful error message with minimum supported API version
• Implements version comparison logic for both legacy and OpenAPI routing
• Adds regression test for version mismatch handling
Diagram
flowchart LR
  A["API Request<br/>with old version"] --> B["Check function<br/>in version"]
  B --> C["Function not found<br/>in version"]
  C --> D["Search all versions<br/>for function"]
  D --> E["Found in newer<br/>version"]
  E --> F["Return helpful error<br/>with min version"]
  D --> G["Not found anywhere"]
  G --> H["Return generic error"]

Loading

File Changes

1. src/node/handler/APIHandler.ts ✨ Enhancement +17/-0

Add version-aware error handling for legacy API routing

• Added compareVersions function to sort API versions semantically
• Implemented logic to detect when function exists in newer API versions
• Returns helpful error message indicating minimum supported API version
• Preserves generic error for truly non-existent functions

src/node/handler/APIHandler.ts


2. src/node/hooks/express/openapi.ts ✨ Enhancement +46/-1

Add version-aware error handling for OpenAPI routing

• Added compareVersions utility function for semantic version sorting
• Added getFunctionNameFromPath helper to extract function names from API paths
• Enhanced notFound handler to detect version mismatches in both FLAT and REST styles
• Returns helpful error message with minimum API version when applicable

src/node/hooks/express/openapi.ts


3. src/tests/backend/specs/api/api.ts 🧪 Tests +15/-0

Add regression test for API version mismatch error

• Added regression test for Issue #6849 covering version mismatch scenario
• Tests /api/1/copyPad endpoint expecting 404 with code 3
• Validates error message indicates minimum supported API version (v1.2.9)

src/tests/backend/specs/api/api.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 23, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. copyPad min version mismatch ✓ Resolved 📎 Requirement gap ≡ Correctness
Description
The new error message and regression test assert that copyPad is only available starting at API
v1.2.9, but the published HTTP API documentation states copyPad is available from API v1.2.8.
This documentation/implementation mismatch can mislead users and breaks the compliance requirement
for consistent API docs.
Code

src/node/handler/APIHandler.ts[R186-191]

Evidence
The compliance rule requires the published API documentation to match the implemented API surface.
The implementation’s version map and new error path indicate copyPad starts at v1.2.9, while the
docs still claim v1.2.8.

API documentation and implementation are consistent for copyPad
src/node/handler/APIHandler.ts[95-99]
src/node/handler/APIHandler.ts[186-191]
doc/api/http_api.md[611-613]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The HTTP API documentation lists `copyPad` as available from API v1.2.8, but the implementation (and new error message/test) indicate availability from v1.2.9.
## Issue Context
- The PR introduces a more helpful 404 message that reports the minimum API version for a function, and the new regression test hard-codes `1.2.9` for `copyPad`.
- The docs under `doc/api/http_api.md` still state `copyPad` is `API >= 1.2.8`, which is inconsistent with the current server-side version map.
## Fix Focus Areas
- doc/api/http_api.md[611-613]
- src/node/handler/APIHandler.ts[95-99]
- src/node/handler/APIHandler.ts[186-191]
- src/tests/backend/specs/api/api.ts[229-231]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. getFunctionNameFromPath() undocumented logic 📘 Rule violation ⚙ Maintainability
Description
New, non-trivial routing logic was added to infer the API function name from the request path
(including REST vs FLAT behavior) without any explanatory comment. This reduces maintainability and
violates the requirement to document complex or non-obvious code.
Code

src/node/hooks/express/openapi.ts[R876-897]

Evidence
The compliance rule requires comments for complex/non-obvious logic. The added helper implements
multi-style path parsing and additional version scanning without documenting assumptions or
rationale.

src/node/hooks/express/openapi.ts[864-897]
Best Practice: Repository guidelines

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New helper logic (`getFunctionNameFromPath`, plus version-sorting behavior) is non-obvious and currently lacks comments explaining intent/assumptions.
## Issue Context
This code is used to generate a more helpful 404 error by detecting when a function exists but is unavailable in the requested API version; future maintainers need to understand why this path-based inference exists and what path formats it supports.
## Fix Focus Areas
- src/node/hooks/express/openapi.ts[864-897]
- src/node/hooks/express/openapi.ts[721-731]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Prototype keys treated as APIs 🐞 Bug ≡ Correctness
Description
getFunctionNameFromPath() uses the in operator to test whether a function exists in
apiHandler.version[v], but those maps are plain objects so inherited keys like
toString/__proto__ will appear to exist. This can return the new “available from API v…” message
for non-existent API functions.
Code

src/node/hooks/express/openapi.ts[R876-892]

Evidence
The OpenAPI helper checks existence using in against plain object maps; because those maps are
created via {}, prototype-chain keys will also satisfy in, leading to false positives.

src/node/hooks/express/openapi.ts[876-896]
src/node/handler/APIHandler.ts[32-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getFunctionNameFromPath()` uses the `in` operator to check for API function existence in `apiHandler.version[v]`. Because these version maps are plain objects, `in` also matches properties from `Object.prototype` (e.g., `toString`, `constructor`, `__proto__`), which can cause invalid function names to be treated as real API operations and trigger the new version-mismatch error path.
## Issue Context
`apiHandler.version` is constructed from `{}` objects, so prototype-chain properties are visible to `in`.
## Fix Focus Areas
- src/node/hooks/express/openapi.ts[876-896]
- src/node/handler/APIHandler.ts[32-36]
### Suggested approach
Replace `funcName in apiHandler.version[v]` with `Object.prototype.hasOwnProperty.call(apiHandler.version[v], funcName)` (or `Object.hasOwn(...)` if available in your TS target), in both FLAT and REST branches.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

4. Version error on method mismatch 🐞 Bug ≡ Correctness
Description
The OpenAPI notFound handler now returns the “available from API v…” message based only on the
path matching a known operation in any version, even though notFound is also used when the HTTP
method is not declared. This can mislead clients into thinking they need a newer API version when
they actually used an unsupported verb for an existing operation in the current version.
Code

src/node/hooks/express/openapi.ts[R721-730]

Evidence
The notFound handler now performs a cross-version existence check and throws the
version-availability message; the test comments explicitly note notFound is used for missing verbs,
so this handler can run in non-version-mismatch scenarios.

src/node/hooks/express/openapi.ts[719-733]
src/node/hooks/express/openapi.ts[739-741]
src/tests/backend/specs/api/api.ts[185-192]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The OpenAPI notFound handler emits a version-mismatch message whenever the request path looks like an API function that exists in *any* version, but `notFound` can also be reached when the request uses an unsupported HTTP method. In that case, the function may already exist in the current mounted API version, making the “available from API vX onwards” message misleading.
## Issue Context
The test suite documents that openapi-backend's `notFound` is used when a method is not declared in the runtime spec.
## Fix Focus Areas
- src/node/hooks/express/openapi.ts[719-733]
- src/node/hooks/express/openapi.ts[739-741]
- src/tests/backend/specs/api/api.ts[185-192]
### Suggested approach
Before throwing the version-mismatch message, ensure the function is actually missing from the current mounted version:
- If `hasOwnProperty(apiHandler.version[version], funcName)` is true, skip the version-mismatch message (optionally return a clearer message such as method not allowed if you can detect it).
- Otherwise, compute `oldestVersion` and only show the helpful message when `compareVersions(version, oldestVersion) < 0`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/node/handler/APIHandler.ts
@JohnMcLear
Copy link
Copy Markdown
Member

JohnMcLear commented May 23, 2026

Why is copyPad moved from 1.2.8 to 1.2.7?
Why have you commented out a process.exit?

Please check your commits before sending PRs or at least check your work after PR :)

cc @SamTV12345 - be careful w/ quality of contributions from Op

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.

copyPad missing from API?

2 participants