From 38d85d4b5a5bf5a680547aeed71e8c98bf14c2a1 Mon Sep 17 00:00:00 2001 From: Leo Q Date: Thu, 28 Mar 2024 22:54:24 +0800 Subject: [PATCH 1/3] provide a bit more info in logs when parsing api schema error When parse error, neither user nor admin can get decent error messages, just `invalid schema`, this is confusing and frustrating, adding a bit more logs could be a good start. --- api/services/tools_manage_service.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/api/services/tools_manage_service.py b/api/services/tools_manage_service.py index 70c6a44459397..c1160f605c402 100644 --- a/api/services/tools_manage_service.py +++ b/api/services/tools_manage_service.py @@ -1,4 +1,5 @@ import json +import logging from flask import current_app from httpx import get @@ -24,6 +25,8 @@ from models.tools import ApiToolProvider, BuiltinToolProvider from services.model_provider_service import ModelProviderService +logger = logging.getLogger(__name__) + class ToolManageService: @staticmethod @@ -309,6 +312,7 @@ def get_api_tool_provider_remote_schema( # try to parse schema, avoid SSRF attack ToolManageService.parser_api_schema(schema) except Exception as e: + logger.error(f"parse api schema error: {str(e)}") raise ValueError('invalid schema, please check the url you provided') return { @@ -655,4 +659,4 @@ def test_api_tool_preview( except Exception as e: return { 'error': str(e) } - return { 'result': result or 'empty response' } \ No newline at end of file + return { 'result': result or 'empty response' } From a7241f2ac13dbadc2ea90a7ba4d03a42cc61df1a Mon Sep 17 00:00:00 2001 From: Leo Q Date: Fri, 29 Mar 2024 05:38:34 +0000 Subject: [PATCH 2/3] Refactor API schema parsing logic --- api/core/tools/utils/parser.py | 131 ++++++++++++--------------------- 1 file changed, 48 insertions(+), 83 deletions(-) diff --git a/api/core/tools/utils/parser.py b/api/core/tools/utils/parser.py index de4ecc8708144..c6d82e5ad4a1a 100644 --- a/api/core/tools/utils/parser.py +++ b/api/core/tools/utils/parser.py @@ -1,10 +1,12 @@ import re import uuid +from json import dumps as json_dumps from json import loads as json_loads +from json.decoder import JSONDecodeError from requests import get -from yaml import FullLoader, load +from yaml import YAMLError, safe_load from core.tools.entities.common_entities import I18nObject from core.tools.entities.tool_bundle import ApiBasedToolBundle @@ -184,27 +186,11 @@ def parse_openapi_yaml_to_tool_bundle(yaml: str, extra_info: dict = None, warnin warning = warning if warning is not None else {} extra_info = extra_info if extra_info is not None else {} - openapi: dict = load(yaml, Loader=FullLoader) + openapi: dict = safe_load(yaml) if openapi is None: raise ToolApiSchemaError('Invalid openapi yaml.') return ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(openapi, extra_info=extra_info, warning=warning) - @staticmethod - def parse_openapi_json_to_tool_bundle(json: str, extra_info: dict = None, warning: dict = None) -> list[ApiBasedToolBundle]: - """ - parse openapi yaml to tool bundle - - :param yaml: the yaml string - :return: the tool bundle - """ - warning = warning if warning is not None else {} - extra_info = extra_info if extra_info is not None else {} - - openapi: dict = json_loads(json) - if openapi is None: - raise ToolApiSchemaError('Invalid openapi json.') - return ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(openapi, extra_info=extra_info, warning=warning) - @staticmethod def parse_swagger_to_openapi(swagger: dict, extra_info: dict = None, warning: dict = None) -> dict: """ @@ -271,38 +257,6 @@ def parse_swagger_to_openapi(swagger: dict, extra_info: dict = None, warning: di return openapi - @staticmethod - def parse_swagger_yaml_to_tool_bundle(yaml: str, extra_info: dict = None, warning: dict = None) -> list[ApiBasedToolBundle]: - """ - parse swagger yaml to tool bundle - - :param yaml: the yaml string - :return: the tool bundle - """ - warning = warning if warning is not None else {} - extra_info = extra_info if extra_info is not None else {} - - swagger: dict = load(yaml, Loader=FullLoader) - - openapi = ApiBasedToolSchemaParser.parse_swagger_to_openapi(swagger, extra_info=extra_info, warning=warning) - return ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(openapi, extra_info=extra_info, warning=warning) - - @staticmethod - def parse_swagger_json_to_tool_bundle(json: str, extra_info: dict = None, warning: dict = None) -> list[ApiBasedToolBundle]: - """ - parse swagger yaml to tool bundle - - :param yaml: the yaml string - :return: the tool bundle - """ - warning = warning if warning is not None else {} - extra_info = extra_info if extra_info is not None else {} - - swagger: dict = json_loads(json) - - openapi = ApiBasedToolSchemaParser.parse_swagger_to_openapi(swagger, extra_info=extra_info, warning=warning) - return ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(openapi, extra_info=extra_info, warning=warning) - @staticmethod def parse_openai_plugin_json_to_tool_bundle(json: str, extra_info: dict = None, warning: dict = None) -> list[ApiBasedToolBundle]: """ @@ -346,40 +300,51 @@ def auto_parse_to_tool_bundle(content: str, extra_info: dict = None, warning: di warning = warning if warning is not None else {} extra_info = extra_info if extra_info is not None else {} - json_possible = False content = content.strip() + loaded_content = None + json_error = None + yaml_error = None + + try: + loaded_content = json_loads(content) + except JSONDecodeError as e: + json_error = e - if content.startswith('{') and content.endswith('}'): - json_possible = True - - if json_possible: - try: - return ApiBasedToolSchemaParser.parse_openapi_json_to_tool_bundle(content, extra_info=extra_info, warning=warning), \ - ApiProviderSchemaType.OPENAPI.value - except: - pass - - try: - return ApiBasedToolSchemaParser.parse_swagger_json_to_tool_bundle(content, extra_info=extra_info, warning=warning), \ - ApiProviderSchemaType.SWAGGER.value - except: - pass - try: - return ApiBasedToolSchemaParser.parse_openai_plugin_json_to_tool_bundle(content, extra_info=extra_info, warning=warning), \ - ApiProviderSchemaType.OPENAI_PLUGIN.value - except: - pass - else: - try: - return ApiBasedToolSchemaParser.parse_openapi_yaml_to_tool_bundle(content, extra_info=extra_info, warning=warning), \ - ApiProviderSchemaType.OPENAPI.value - except: - pass - + if loaded_content is None: try: - return ApiBasedToolSchemaParser.parse_swagger_yaml_to_tool_bundle(content, extra_info=extra_info, warning=warning), \ - ApiProviderSchemaType.SWAGGER.value - except: - pass + loaded_content = safe_load(content) + except YAMLError as e: + yaml_error = e + if loaded_content is None: + raise ToolApiSchemaError(f'Invalid api schema, schema is neither json nor yaml. json error: {str(json_error)}, yaml error: {str(yaml_error)}') + + swagger_error = None + openapi_error = None + openapi_plugin_error = None + schema_type = None + + try: + openapi = ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(loaded_content, extra_info=extra_info, warning=warning) + schema_type = ApiProviderSchemaType.OPENAPI.value + return openapi, schema_type + except ToolApiSchemaError as e: + openapi_error = e + + # openai parse error, fallback to swagger + # convert to openapi, if faield, raise Exception directly + converted_swagger = ApiBasedToolSchemaParser.parse_swagger_to_openapi(loaded_content, extra_info=extra_info, warning=warning) + schema_type = ApiProviderSchemaType.SWAGGER.value + + try: + return ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(converted_swagger, extra_info=extra_info, warning=warning), schema_type + except ToolApiSchemaError as e: + openapi_error = e + + try: + openapi_plugin = ApiBasedToolSchemaParser.parse_openai_plugin_json_to_tool_bundle(json_dumps(loaded_content), extra_info=extra_info, warning=warning) + return openapi_plugin, ApiProviderSchemaType.OPENAI_PLUGIN.value + except ToolNotSupportedError as e: + # maybe it's not plugin at all + openapi_plugin_error = e - raise ToolApiSchemaError('Invalid api schema.') \ No newline at end of file + raise ToolApiSchemaError(f'Invalid api schema, openapi error: {str(openapi_error)}, swagger error: {str(swagger_error)}, openapi plugin error: {str(openapi_plugin_error)}') From 6ada9402b77325e15b053eb2b2c10b6ecdb55fd6 Mon Sep 17 00:00:00 2001 From: Leo Q Date: Fri, 29 Mar 2024 05:50:55 +0000 Subject: [PATCH 3/3] Refactor API schema parsing logic --- api/core/tools/utils/parser.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api/core/tools/utils/parser.py b/api/core/tools/utils/parser.py index c6d82e5ad4a1a..5efd2e49b9812 100644 --- a/api/core/tools/utils/parser.py +++ b/api/core/tools/utils/parser.py @@ -331,15 +331,14 @@ def auto_parse_to_tool_bundle(content: str, extra_info: dict = None, warning: di openapi_error = e # openai parse error, fallback to swagger - # convert to openapi, if faield, raise Exception directly - converted_swagger = ApiBasedToolSchemaParser.parse_swagger_to_openapi(loaded_content, extra_info=extra_info, warning=warning) - schema_type = ApiProviderSchemaType.SWAGGER.value - try: + converted_swagger = ApiBasedToolSchemaParser.parse_swagger_to_openapi(loaded_content, extra_info=extra_info, warning=warning) + schema_type = ApiProviderSchemaType.SWAGGER.value return ApiBasedToolSchemaParser.parse_openapi_to_tool_bundle(converted_swagger, extra_info=extra_info, warning=warning), schema_type except ToolApiSchemaError as e: - openapi_error = e + swagger_error = e + # swagger parse error, fallback to openai plugin try: openapi_plugin = ApiBasedToolSchemaParser.parse_openai_plugin_json_to_tool_bundle(json_dumps(loaded_content), extra_info=extra_info, warning=warning) return openapi_plugin, ApiProviderSchemaType.OPENAI_PLUGIN.value