Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
christiankaindl opened this issue Oct 7, 2024 · 2 comments
Open

Comments

@christiankaindl
Copy link

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
Copy link
Contributor

tharropoulos commented Oct 8, 2024

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
Copy link
Member

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.

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

No branches or pull requests

3 participants