typesense / typesense-js

JavaScript / TypeScript client for Typesense
https://typesense.org/docs/api
Apache License 2.0
411 stars 75 forks source link

Confusing error when passing invalid ID to synonym `upsert()` #233

Open christiankaindl opened 1 week ago

christiankaindl commented 1 week ago

Description

When using the Synonym upsert() function in the Typesense JS SDK, invalid IDs throw an ambiguous error which doesn't give any clue on what the issue is or how to fix it.

Steps to reproduce

await typesenseClient.collections('myCollection').synonyms().upsert('Ärztinnen-/Ärzte-Ausbildungsordnung 2015', {
  synonyms: ['Syn 1', 'Syn 2'],
})

Edit: After some more testing, it seems that the slash ("/") is the problem, using umlaute ("ä") or non-breaking spaces as the ID seems to work just fine.

Expected Behavior

If the ID needs to match a certain format, I'd expect a informative error to be thrown, something the lines of "Invalid ID. IDs have to match format ".

As an alternative, invalid IDs could automatically be coerced to a valid one, or given a UUID. Though a useful error is probably more expected.

Actual Behavior

I get an error that caused us some confusion on how to fix it:

ObjectNotFound: Request failed with HTTP code 404 | Server said: Not Found
    at ObjectNotFound.TypesenseError [as constructor] (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/src/Typesense/Errors/TypesenseError.ts:6:10)
    at new ObjectNotFound (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/lib/Typesense/Errors/ObjectNotFound.js:25:42)
    at ApiCall.customErrorForResponse (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/src/Typesense/ApiCall.ts:424:15)
    at /Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/src/Typesense/ApiCall.ts:260:18
    at step (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/lib/Typesense/ApiCall.js:33:23)
    at Object.next (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/lib/Typesense/ApiCall.js:14:53)
    at step (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/lib/Typesense/ApiCall.js:18:139)
    at Object.next (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/lib/Typesense/ApiCall.js:14:53)
    at fulfilled (/Users/christiankaindl/Documents/GesetzeFinden.at/backend/node_modules/typesense/lib/Typesense/ApiCall.js:5:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  httpStatus: 404
}

Metadata

Typesense Version: Server 27.1; JS SDK 1.8.2

OS: macOS 15.0

tharropoulos commented 1 week ago

The problem as you correctly mentioned is the / character. Since the library is a thin wrapper around the API, you'd be sending a request that uses another endpoint. So the server in turn will return a 404 for that request. Adding another level of runtime checks specifically for sanitizing the input for operations like this one is doable, and while not too keen on it myself, it's up to @jasonbosco to decide if we add it in

jasonbosco commented 1 week ago

Could you try this with v2.0.0-6 of typesense-js? We automatically url-encode special characters in that version, so this shouldn't happen.