feat(wikibase): handle not found errors for properties and items#155
feat(wikibase): handle not found errors for properties and items#155
Conversation
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.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds explicit Changes
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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. Comment |
There was a problem hiding this comment.
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
Inconstraint-validation.service.ts,getPropertycan returnnullbut its result is passed directly intoparseConstraintsFromProperty(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 blockThis 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 itemsSymmetric 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
📒 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}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript or JavaScript files
Do not use dotenv; Bun automatically loads .env files
UseBun.serve()for HTTP servers and WebSockets instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofws
PreferBun.fileovernode:fs's readFile/writeFile
UseBun.$(e.g.,Bun.$ls``) instead of execa for running shell commands
Files:
backend/tests/api/wikibase/entities.test.tsbackend/src/api/wikibase/entities.tsbackend/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 testinstead ofjestfor 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 ofwebpackoresbuildfor building HTML, TypeScript, or CSS files
Files:
backend/tests/api/wikibase/entities.test.tsbackend/src/api/wikibase/entities.tsbackend/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 correctMatches the updated service contract and test expectations.
92-96: Item not-found handling mirrors propertiesConsistent 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' |
There was a problem hiding this comment.
💡 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=tsxLength 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), replacewithimport Elysia from 'elysia'
import { Elysia } from 'elysia'
- In
backend/tests/api/wikibase/constraints.test.ts(line 5), replacewithimport Elysia from 'elysia'
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.
| // Mock the service | ||
| Object.assign(wikibaseService, mockWikibaseService) | ||
|
|
There was a problem hiding this comment.
🛠️ 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.
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:
getPropertyandgetItemmethods in theWikibaseServicenow returnnullinstead of throwing an error when the property or item is not found.wikibase.entities.propertiesandwikibase.entities.itemsAPI endpoints have been updated to handle thenullreturn 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.