verida / data-connector-server

1 stars 2 forks source link

Handle potential invalid promptSearchResult #156

Open aurelticot opened 3 weeks ago

aurelticot commented 3 weeks ago

When calling llm/personal with a simple prompt, I got a 500 because databases was undefined on the following line.

https://github.com/verida/data-connector-server/blob/3156b74ea5a8951abf868184a0d85fd942c6f273/src/services/assistants/search.ts#L49

Here are the logs

[2024-10-31T15:01:54.612] [INFO] default - {
  search_type: 'keywords',
  keywords: [ 'greeting', 'hello' ],
  timeframe: null,
  databases: null,
  sort: null,
  output_type: null,
  profile_information: null
}
[2024-10-31T15:01:54.613] [INFO] default - Searching by keywords: greeting,hello
[2024-10-31T15:01:54.613] [INFO] default - TypeError: Cannot read properties of null (reading 'indexOf')
    at PromptSearchService.<anonymous> (/Users/aurel/dev/verida/data-connector-server/dist/services/assistants/search.js:46:50)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/aurel/dev/verida/data-connector-server/dist/services/assistants/search.js:5:58)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

promptSearchResult comes from

https://github.com/verida/data-connector-server/blob/3156b74ea5a8951abf868184a0d85fd942c6f273/src/services/tools/promptSearch.ts#L58

The assertion of a JSON.parse is error-prone. In that case it was assumed databases (and other property) is defined as per the PromptSearchLLMResponse type but it was not.

The assertion is not the root cause of why databases is null but that doesn't help in static code analysis.

tahpot commented 2 weeks ago

Yes, I'm aware this isn't great.

I want to move to using zod to verify the LLM response along with user input, but haven't got to that yet.

For now I'll add some basic checks, but will leave this issue open until we do something more robust.