AI-Proxy plugin: "An unexpected error occurred" when upstream URL is missing port/path

dascole commented 6 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Kong version (`$ kong version`)

3.6.1

Current Behavior

When the AI-Proxy plugin is enabled and the configured to use Model.Options.Upstream Url that lacks a port/path, the below behavior is observed.

Missing Path: If the upstream URL is missing a path, an HTTP 500 is returned with the body “An unexpected error occurred”. The corresponding log entry shows the cause.

2024/04/16 13:58:38 [error] 1279#0: *20351 [kong] init.lua:405 [ai-proxy] /usr/local/share/lua/5.1/kong/llm/drivers/llama2.lua:266: path must be a string, client: 172.25.0.1, server: kong, request: "POST /echo HTTP/1.1", host: "localhost:8000", request_id: "3c5c232285a29358a4d7567573a04219"

Missing Port: Similarly, if the upstream URL is missing the port, it results in another HTTP 500 error, with the log showing

2024/04/16 13:59:23 [error] 1279#0: *20271 [kong] init.lua:405 [ai-proxy] /usr/local/share/lua/5.1/kong/llm/drivers/llama2.lua:268: port must be an integer, client: 172.25.0.1, server: kong, request: "POST /echo HTTP/1.1", host: "localhost:8000", request_id: "9a948902ff31d42d49899041b1079f5f"

These are the result of passing nil values to

kong.service.request.set_path(parsed_url.path)
kong.service.set_target(parsed_url.host, tonumber(parsed_url.port))

Expected Behavior

The AI-Proxy plugin should handle cases where the components of the upstream URL are missing more gracefully:

Missing Path: When the upstream URL lacks a path, the plugin should default to using “/” instead of throwing a server error. This would provide a fallback behavior to maintaining functionality.
Missing Port: If the upstream URL does not specify a port, the plugin should infer the port based on the protocol used (i.e HTTP/80, HTTPS/443).

Steps To Reproduce

Spin up the Kong instance

version: "3.7"

volumes:
    kong_data_ce: {}

services:
  kong-postgres-ce:
    image: postgres:9.6
    restart: always
    container_name: kong-postgres-ce
    networks:
      - kong-ce
    environment:
      POSTGRES_USER: kong
      POSTGRES_PASSWORD: kong
      POSTGRES_HOST_AUTH_METHOD: trust
      POSTGRES_DB: kong
    ports:
      - "7432:5432/tcp"

  kong-migrations-ce:
    image: kong/kong:3.6.1
    container_name: kong-migrations-ce
    command: kong migrations bootstrap
    depends_on:
      - kong-postgres-ce
    environment:
      KONG_DATABASE: postgres
      KONG_PASSWORD: admin
      KONG_PG_DATABASE: ${KONG_PG_DATABASE:-kong}
      KONG_PG_HOST: kong-postgres-ce
      KONG_PG_PASSWORD: ${KONG_PG_PASSWORD:-kong}
      KONG_PG_USER: ${KONG_PG_USER:-kong}
    networks:
      - kong-ce
    restart: on-failure

  kong-ce:
    image: kong/kong:3.6.1
    container_name: kong-ce
    networks:
      - kong-ce
    depends_on:
      - kong-postgres-ce
      - kong-migrations-ce
    restart: on-failure
    ports:
      - "8000:8000/tcp"
      - "8001:8001/tcp"
      - "8002:8002/tcp"
      - "8443:8443/tcp"
      - "8444:8444/tcp"
      - "8445:8445/tcp"
    environment:
      KONG_PG_DATABASE: ${KONG_PG_DATABASE:-kong}
      KONG_ADMIN_GUI_LISTEN:  0.0.0.0:8002, 0.0.0.0:8445 ssl http2
      KONG_PROXY_LISTEN: 0.0.0.0:8000, 0.0.0.0:8443 ssl
      KONG_ADMIN_LISTEN: 0.0.0.0:8001, 0.0.0.0:8444 ssl http2
      KONG_LOG_LEVEL: debug
      KONG_DATABASE: postgres
      KONG_PG_HOST: kong-postgres-ce
      KONG_PG_PASSWORD: ${KONG_PG_PASSWORD:-kong}
      KONG_PG_USER: ${KONG_PG_USER:-kong}

networks:
  kong-ce:
    name: kong-ce
    driver: bridge

Sync the deck config: deck gateway sync --kong-addr http://localhost:8001 kong.yaml

_format_version: "3.0"
services:
- connect_timeout: 60000
  enabled: true
  host: httpbin.org
  name: httpbin
  path: /anything
  port: 443
  protocol: https
  read_timeout: 60000
  retries: 5
  routes:
  - https_redirect_status_code: 426
    name: httpbin-route
    path_handling: v0
    paths:
    - /echo
    plugins:
    - config:
        auth: null
        logging:
          log_payloads: false
          log_statistics: true
        model:
          name: llama2
          options:
            anthropic_version: null
            azure_api_version: "2023-05-15"
            azure_deployment_id: null
            azure_instance: null
            llama2_format: ollama
            max_tokens: 256
            mistral_format: null
            temperature: 1
            top_k: 0
            top_p: 1
            upstream_url: https://localhost
          provider: llama2
        route_type: llm/v1/chat
      enabled: true
      name: ai-proxy
      protocols:
      - grpc
      - grpcs
      - http
      - https
    preserve_host: false
    protocols:
    - http
    - https
    regex_priority: 0
    request_buffering: true
    response_buffering: true
    strip_path: true
  write_timeout: 60000

Send a request

curl --request POST http://localhost:8000/echo \
  --header 'Content-Type: application/json' \
  --data '{
    "messages": [
        {
            "role":"system",
            "content":"You are a mathematician"
        },
        {
            "role":"user",
            "content":"What is 1+1?"
        }

    ]

}'

Response:

{
  "message":"An unexpected error occurred",
  "request_id":"56d0493b6ca4b81cf5cc942c2204815f"
}

Review the logs

docker logs -f kong-ce 2>&1 |grep -i error

2024/04/16 16:20:54 [error] 1328#0: *12987 [kong] init.lua:405 [ai-proxy] /usr/local/share/lua/5.1/kong/llm/drivers/llama2.lua:266: path must be a string, client: 172.27.0.1, server: kong, request: "POST /echo HTTP/1.1", host: "localhost:8000", request_id: "56d0493b6ca4b81cf5cc942c2204815f"

Anything else?

I'm happy to submit the PR for this if we agree on the approach. My thought is to simply perform the checks mentioned above, something along these lines.

This has been working well in testing, but open to other ideas and would like to help drive this forward.

 parsed_url.path = parsed_url.path or "/"

    if not parsed_url.port then
      if parsed_url.scheme == "https" then
        parsed_url.port = 443
      else
        parsed_url.port = 80
      end
    end

Water-Melon commented 6 months ago

It is tracked by KAG-4312

rohitrsh commented 6 months ago

Hey @Water-Melon, I don't have access to Kong JIRA. How can I get updates on KAG-4312? I am also facing same issue with self hosted llama2 model

chronolaw commented 6 months ago

https://github.com/Kong/kong/pull/12903 may fix this issue, could you check it? thanks.

dascole commented 6 months ago

@chronolaw I have tested against this branch but it throws another 500

{
    "message": "An unexpected error occurred",
    "request_id": "b1dc385ad051903a0a30441d9b7dc97d"
}

2024/05/06 10:24:52 [error] 2660914#0: *12887 [kong] init.lua:405 [ai-proxy] ./kong/llm/drivers/llama2.lua:267: bad argument #1 to 'string_gsub' (string expected, got nil), client: 127.0.0.1, server: kong, request: "POST /echo HTTP/1.1", host: "localhost:8000", request_id: "b1dc385ad051903a0a30441d9b7dc97d"

When no path is specified parsed_url.path is nil

2024/05/06 10:24:52 [notice] 2660914#0: *12887 [lua] llama2.lua:265: configure_request(): parsed_url = {
  authority = "myserver:11434",
  host = "myserver",
  port = "11434",
  scheme = "http"
}

$ kong version
3.7.0

$ git branch
* feat/streamline_ai_proxy

chronolaw commented 6 months ago

Thank you @dascole , we have recorded it in the ticket KAG-4312, we are still trying to fix it.

tysoekong commented 6 months ago

Hi everyone; many apologies for this bug - I had originally fixed it and I knew it was coming in 3.7.0, so I wasn't watching this issue, and now during refactoring the URL parser for fixing OLLAMA token streaming I had broken it again. This is my fault here.

I have opened the suggested fix and tested it again: https://github.com/Kong/kong/pull/12998 but we may be frozen for 3.7.0 features now, so it might roll into fixes for 3.7.1 instead.

See the linked pull request for updates on this one.

tysoekong commented 4 months ago

Hi all @chronolaw @dascole @rohitrsh @Water-Melon

This is fixed in Kong 3.7.0 and onwards, have verified it's working with/without port, path, etc.

Kong / kong