Skip to content

feat(wikibase): handle not found errors for properties and items#155

Merged
DaxServer merged 3 commits intomainfrom
404-notfound
Sep 8, 2025
Merged

feat(wikibase): handle not found errors for properties and items#155
DaxServer merged 3 commits intomainfrom
404-notfound

Conversation

@DaxServer
Copy link
Owner

The changes in this commit address the handling of not found errors for Wikibase properties and items. Previously, the API was throwing errors when a property or item was not found, which could lead to unexpected behavior.

The main changes are:

  1. The getProperty and getItem methods in the WikibaseService now return null instead of throwing an error when the property or item is not found.
  2. The wikibase.entities.properties and wikibase.entities.items API endpoints have been updated to handle the null return value from the service methods. They now return a 404 error response with the appropriate error message when the property or item is not found.

These changes improve the overall error handling and user experience of the Wikibase API.

The changes in this commit address the handling of not found errors for Wikibase properties and items. Previously, the API was throwing errors when a property or item was not found, which could lead to unexpected behavior. 

The main changes are:

1. The `getProperty` and `getItem` methods in the `WikibaseService` now return `null` instead of throwing an error when the property or item is not found.
2. The `wikibase.entities.properties` and `wikibase.entities.items` API endpoints have been updated to handle the `null` return value from the service methods. They now return a 404 error response with the appropriate error message when the property or item is not found.

These changes improve the overall error handling and user experience of the Wikibase API.
@coderabbitai
Copy link

coderabbitai bot commented Sep 8, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • API now returns 404 Not Found with a structured error payload when requesting non-existent Wikibase items or properties, providing clear messages (e.g., “Item with identifier 'Q1000000' not found”).
    • Improved consistency of HTTP status handling across these endpoints.
  • Tests

    • Added tests verifying 404 responses and error payloads for missing items and properties.

Walkthrough

Adds explicit status handling to Wikibase entity route handlers, changes not-found handling to use ApiErrorHandler.notFoundError(type, id) and respond with status(404, ...), updates WikibaseService.getProperty/getItem to return null for missing entities, and adds tests that assert 404 payloads for not-found cases.

Changes

Cohort / File(s) Summary
API handlers: Wikibase entities
backend/src/api/wikibase/entities.ts
Route handlers for /properties/:propertyId and /items/:itemId now destructure status from context. Not-found handling uses ApiErrorHandler.notFoundError(type, id) and responds via status(404, ...). Core happy-path logic unchanged.
Service layer: WikibaseService
backend/src/services/wikibase.service.ts
getProperty and getItem signatures changed to return `Promise<...
Tests: Wikibase entities API
backend/tests/api/wikibase/entities.test.ts
Adds tests mocking wikibaseService to return null, asserting 404 responses and structured error payloads for missing properties/items (includes a duplicated item 404 test block).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API as Wikibase Entities API
    participant Service as WikibaseService

    Client->>API: GET /properties/:id or /items/:id
    API->>Service: getProperty/getItem(instanceId, id)
    alt found
        Service-->>API: entity
        API-->>Client: 200 OK (entity payload)
    else not found (null)
        Service-->>API: null
        API->>API: ApiErrorHandler.notFoundError(type, id)
        API-->>Client: 404 Not Found (structured error) 
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49b2f4c and ddffdd0.

📒 Files selected for processing (2)
  • backend/src/services/wikibase.service.ts (4 hunks)
  • backend/tests/api/wikibase/entities.test.ts (1 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 404-notfound

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/src/services/wikibase.service.ts (1)

156-175: Detect missing null-check in constraint validation
In constraint-validation.service.ts, getProperty can return null but its result is passed directly into parseConstraintsFromProperty (line 31) without a guard—this will throw at runtime when the property isn’t found. Add a null check before parsing.

🧹 Nitpick comments (2)
backend/tests/api/wikibase/entities.test.ts (1)

114-133: Remove duplicate 404 item test block

This block duplicates the preceding “Item endpoints” 404 test.

Apply:

-  describe('Item endpoints', () => {
-    test('should return 404 for non-existent item', async () => {
-      const { data, status, error } = await api.wikibase.entities.items({ itemId: 'Q1000000' }).get({
-        query: { instance: 'wikidata' },
-      })
-
-      expect(data).toBeNull()
-      expect(status).toBe(404)
-      expect(error).toHaveProperty('value', {
-          data: [],
-          errors: [
-            {
-              details: [],
-              code: 'NOT_FOUND',
-              message: "Item with identifier 'Q1000000' not found",
-            },
-          ],
-        })
-    })
-
-    test('should return 200 for existing item', async () => {
+    test('should return 200 for existing item', async () => {
       const { data, status, error } = await api.wikibase.entities.items({ itemId: 'Q1' }).get({
         query: { instance: 'wikidata' },
       })
@@
-    })
-  })
+    })
backend/src/services/wikibase.service.ts (1)

266-285: Consistent null return for missing items

Symmetric handling with properties; good.

Add TSDoc noting the nullable return to aid callers and tooling.

@@
-  /**
-   * Get property details by ID
-   */
+  /**
+   * Get property details by ID.
+   * Returns null when the entity is missing (HTTP 200 with `missing` flag).
+   */
@@
-  /**
-   * Get item details by ID
-   */
+  /**
+   * Get item details by ID.
+   * Returns null when the entity is missing (HTTP 200 with `missing` flag).
+   */
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6146a9 and 49b2f4c.

📒 Files selected for processing (3)
  • backend/src/api/wikibase/entities.ts (2 hunks)
  • backend/src/services/wikibase.service.ts (4 hunks)
  • backend/tests/api/wikibase/entities.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

**/*.{ts,tsx,js,jsx}: Use bun <file> instead of node <file> or ts-node <file> for running TypeScript or JavaScript files
Do not use dotenv; Bun automatically loads .env files
Use Bun.serve() for HTTP servers and WebSockets instead of express
Use bun:sqlite for SQLite instead of better-sqlite3
Use Bun.redis for Redis instead of ioredis
Use Bun.sql for Postgres instead of pg or postgres.js
Use built-in WebSocket instead of ws
Prefer Bun.file over node:fs's readFile/writeFile
Use Bun.$ (e.g., Bun.$ls``) instead of execa for running shell commands

Files:

  • backend/tests/api/wikibase/entities.test.ts
  • backend/src/api/wikibase/entities.ts
  • backend/src/services/wikibase.service.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Use bun test instead of jest for running tests

Files:

  • backend/tests/api/wikibase/entities.test.ts
**/*.{html,ts,tsx,css}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Use bun build <file.html|file.ts|file.css> instead of webpack or esbuild for building HTML, TypeScript, or CSS files

Files:

  • backend/tests/api/wikibase/entities.test.ts
  • backend/src/api/wikibase/entities.ts
  • backend/src/services/wikibase.service.ts
🧬 Code graph analysis (3)
backend/tests/api/wikibase/entities.test.ts (2)
backend/src/services/wikibase.service.ts (1)
  • wikibaseService (348-348)
backend/src/api/wikibase/entities.ts (1)
  • wikibaseEntitiesApi (14-100)
backend/src/api/wikibase/entities.ts (1)
backend/src/types/error-handler.ts (1)
  • ApiErrorHandler (6-211)
backend/src/services/wikibase.service.ts (1)
backend/src/types/wikibase-api.ts (2)
  • PropertyDetails (21-30)
  • ItemDetails (51-56)
🔇 Additional comments (2)
backend/src/api/wikibase/entities.ts (2)

52-56: Use of status(404, …) with structured NOT_FOUND payload is correct

Matches the updated service contract and test expectations.


92-96: Item not-found handling mirrors properties

Consistent and clear.

import { wikibaseService } from '@backend/services/wikibase.service'
import { treaty } from '@elysiajs/eden'
import { beforeEach, describe, expect, test } from 'bun:test'
import Elysia from 'elysia'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify Elysia import style

Confirm that a default import from 'elysia' is valid in your setup. If not, switch to a named import to match the rest of the codebase.

Run to check consistency:


🏁 Script executed:

#!/bin/bash
rg -nP -C2 "import\s+Elysia\s+from\s+'elysia'|import\s+\{\s*Elysia\s*\}\s+from\s+'elysia'" --type=ts --type=tsx

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Search for both default and named Elysia imports without relying on file-type filters
rg -nP -C2 "import\s+(?:Elysia|\{\s*Elysia\s*\})\s+from\s+'elysia'" -g '*.ts' -g '*.tsx'

Length of output: 6361


Use named Elysia import for consistency

  • In backend/tests/api/wikibase/entities.test.ts (line 5), replace
    import Elysia from 'elysia'
    with
    import { Elysia } from 'elysia'
  • In backend/tests/api/wikibase/constraints.test.ts (line 5), replace
    import Elysia from 'elysia'
    with
    import { Elysia } from 'elysia'
🤖 Prompt for AI Agents
In backend/tests/api/wikibase/entities.test.ts (line 5) and
backend/tests/api/wikibase/constraints.test.ts (line 5), the module is imported
as a default export; change each to use the named Elysia import from 'elysia'
(i.e., replace the default import with a named import) so imports are consistent
across tests, then run lint/tests to ensure no import-related failures.

Comment on lines +38 to +40
// Mock the service
Object.assign(wikibaseService, mockWikibaseService)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Restore wikibaseService after tests to avoid cross-suite pollution

Overriding the singleton globally can leak into other tests. Save originals and restore them after the suite.

Apply:

@@
-import { beforeEach, describe, expect, test } from 'bun:test'
+import { beforeEach, afterAll, describe, expect, test } from 'bun:test'
@@
-// Mock the service
-Object.assign(wikibaseService, mockWikibaseService)
+// Preserve originals and mock the service
+const originalWikibaseService = {
+  getProperty: wikibaseService.getProperty.bind(wikibaseService),
+  getItem: wikibaseService.getItem.bind(wikibaseService),
+}
+Object.assign(wikibaseService, mockWikibaseService)
@@
 describe('Wikibase Entities API', () => {
   let api: ReturnType<typeof createTestApi>
@@
   beforeEach(() => {
     api = createTestApi()
   })
+
+  afterAll(() => {
+    Object.assign(wikibaseService, originalWikibaseService)
+  })

Also applies to: 48-50

🤖 Prompt for AI Agents
In backend/tests/api/wikibase/entities.test.ts around lines 38-40 (and also
48-50), the test suite overwrites the singleton wikibaseService directly which
can leak into other test suites; modify the file to save the original service
reference before assigning the mock (e.g., const originalWikibaseService = {
...wikibaseService } or const originalWikibaseService = wikibaseService), then
assign the mock for the suite, and restore the original in an afterAll (or
afterEach) hook by reassigning the saved reference back to wikibaseService so
the global singleton is returned to its original state after tests complete.

@DaxServer DaxServer merged commit 0243a0c into main Sep 8, 2025
4 checks passed
@DaxServer DaxServer deleted the 404-notfound branch September 8, 2025 18:01
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.

1 participant