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

READY: Consolidate error responses #8264

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/tribler/core/content_discovery/restapi/search_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,16 @@ async def remote_search(self, request: RequestType) -> RESTResponse:
try:
sanitized = DatabaseEndpoint.sanitize_parameters(request.query)
except (ValueError, KeyError) as e:
return RESTResponse({"error": f"Error processing request parameters: {e}"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": f"Error processing request parameters: {e}"
}}, status=HTTP_BAD_REQUEST)
query = request.query.get("fts_text")
if query is None:
return RESTResponse({"error": f"Got search with no fts_text: {dict(request.query)}"},
return RESTResponse({"error": {
"handled": True,
"message": f"Got search with no fts_text: {dict(request.query)}"
}},
status=HTTP_BAD_REQUEST)
if t_filter := request.query.get("filter"):
query += f" {t_filter}"
Expand Down
26 changes: 20 additions & 6 deletions src/tribler/core/database/restapi/database_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,10 @@ async def get_torrent_health(self, request: RequestType) -> RESTResponse:
try:
timeout = int(request.query.get("timeout", TORRENT_CHECK_TIMEOUT))
except ValueError as e:
return RESTResponse({"error": f"Error processing timeout parameter: {e}"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": f"Error processing timeout parameter: {e}"
}}, status=HTTP_BAD_REQUEST)

if self.torrent_checker is None:
return RESTResponse({"checking": False})
Expand Down Expand Up @@ -270,16 +273,24 @@ async def local_search(self, request: RequestType) -> RESTResponse: # noqa: C90
sanitized = self.sanitize_parameters(request.query)
tags = sanitized.pop("tags", None)
except (ValueError, KeyError):
return RESTResponse({"error": "Error processing request parameters"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "Error processing request parameters"
}}, status=HTTP_BAD_REQUEST)

if self.tribler_db is None:
return RESTResponse({"error": "Tribler DB not initialized"}, status=HTTP_NOT_FOUND)
return RESTResponse({"error": {
"handled": True,
"message": "Tribler DB not initialized"
}}, status=HTTP_NOT_FOUND)

include_total = request.query.get("include_total", "")
query = request.query.get("fts_text")
if query is None:
return RESTResponse({"error": f"Got search with no fts_text: {dict(request.query)}"},
status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": f"Got search with no fts_text: {dict(request.query)}"
}}, status=HTTP_BAD_REQUEST)
if t_filter := request.query.get("filter"):
query += f" {t_filter}"
fts = to_fts_query(query)
Expand Down Expand Up @@ -354,7 +365,10 @@ async def completions(self, request: RequestType) -> RESTResponse:
"""
args = request.query
if "q" not in args:
return RESTResponse({"error": "query parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "query parameter missing"
}}, status=HTTP_BAD_REQUEST)

keywords = args["q"].strip().lower()
results = request.context[0].get_auto_complete_terms(keywords, max_terms=5)
Expand Down
21 changes: 16 additions & 5 deletions src/tribler/core/knowledge/restapi/knowledge_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,15 @@ def validate_infohash(infohash: str) -> tuple[bool, RESTResponse | None]:
"""
try:
if len(infohash) != 40:
return False, RESTResponse({"error": "Invalid infohash"}, status=HTTP_BAD_REQUEST)
return False, RESTResponse({"error": {
"handled": True,
"message": "Invalid infohash"
}}, status=HTTP_BAD_REQUEST)
except binascii.Error:
return False, RESTResponse({"error": "Invalid infohash"}, status=HTTP_BAD_REQUEST)
return False, RESTResponse({"error": {
"handled": True,
"message": "Invalid infohash"
}}, status=HTTP_BAD_REQUEST)

return True, None

Expand All @@ -77,7 +83,8 @@ def validate_infohash(infohash: str) -> tuple[bool, RESTResponse | None]:
"schema": schema(UpdateTagsResponse={"success": Boolean()})
},
HTTP_BAD_REQUEST: {
"schema": HandledErrorSchema, "example": {"error": "Invalid tag length"}},
"schema": HandledErrorSchema, "example": {"error": {"handled": True, "message": "Invalid tag length"}}
}
},
description="This endpoint updates a particular torrent with the provided metadata."
)
Expand All @@ -97,7 +104,10 @@ async def update_knowledge_entries(self, request: RequestType) -> RESTResponse:
for statement in params["statements"]:
obj = statement["object"]
if not is_valid_resource(obj):
return RESTResponse({"error": "Invalid tag length"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "Invalid tag length"
}}, status=HTTP_BAD_REQUEST)

statements.append(statement)

Expand Down Expand Up @@ -146,7 +156,8 @@ def modify_statements(self, db: TriblerDatabase, infohash: str, statements: list
"schema": schema(SuggestedTagsResponse={"suggestions": List(String)})
},
HTTP_BAD_REQUEST: {
"schema": HandledErrorSchema, "example": {"error": "Invalid infohash"}},
"schema": HandledErrorSchema, "example": {"error": {"handled": True, "message": "Invalid infohash"}}
}
},
description="This endpoint updates a particular torrent with the provided tags."
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def __init__(self, download_manager: DownloadManager, client_max_size: int = MAX
},
HTTP_BAD_REQUEST: {
"schema": HandledErrorSchema,
"examples": {"Error": {"error": "files parameter missing"}}
"examples": {"Error": {"error": {"handled": True, "message": "files parameter missing"}}}
}
}
)
Expand All @@ -96,7 +96,10 @@ async def create_torrent(self, request: Request) -> RESTResponse:
if parameters.get("files"):
file_path_list = [Path(p) for p in parameters["files"]]
else:
return RESTResponse({"error": "files parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "files parameter missing"
}}, status=HTTP_BAD_REQUEST)

if parameters.get("description"):
params["comment"] = parameters["description"]
Expand Down
97 changes: 76 additions & 21 deletions src/tribler/core/libtorrent/restapi/downloads_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ def return_404(message: str = "this download does not exist") -> RESTResponse:
"""
Returns a 404 response code if your channel has not been created.
"""
return RESTResponse({"error": message}, status=HTTP_NOT_FOUND)
return RESTResponse({"error": {
"handled": True,
"message": message
}}, status=HTTP_NOT_FOUND)

def create_dconfig_from_params(self, parameters: dict) -> tuple[DownloadConfig, None] | tuple[None, str]:
"""
Expand Down Expand Up @@ -423,19 +426,28 @@ async def add_download(self, request: Request) -> RESTResponse: # noqa: C901
params = await request.json()
uri = params.get("uri")
if not uri:
return RESTResponse({"error": "uri parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "uri parameter missing"
}}, status=HTTP_BAD_REQUEST)

download_config, error = self.create_dconfig_from_params(params)
if error:
return RESTResponse({"error": error}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": error
}}, status=HTTP_BAD_REQUEST)

try:
if tdef:
download = await self.download_manager.start_download(tdef=tdef, config=download_config)
elif uri:
download = await self.download_manager.start_download_from_uri(uri, config=download_config)
except Exception as e:
return RESTResponse({"error": str(e)}, status=HTTP_INTERNAL_SERVER_ERROR)
return RESTResponse({"error": {
"handled": True,
"message": str(e)
}}, status=HTTP_INTERNAL_SERVER_ERROR)

return RESTResponse({"started": True, "infohash": hexlify(download.get_def().get_infohash()).decode()})

Expand Down Expand Up @@ -465,7 +477,10 @@ async def delete_download(self, request: Request) -> RESTResponse:
"""
parameters = await request.json()
if "remove_data" not in parameters:
return RESTResponse({"error": "remove_data parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "remove_data parameter missing"
}}, status=HTTP_BAD_REQUEST)

infohash = unhexlify(request.match_info["infohash"])
download = self.download_manager.get_download(infohash)
Expand Down Expand Up @@ -514,8 +529,10 @@ async def update_download(self, request: Request) -> RESTResponse: # noqa: C901

parameters = await request.json()
if len(parameters) > 1 and "anon_hops" in parameters:
return RESTResponse({"error": "anon_hops must be the only parameter in this request"},
status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "anon_hops must be the only parameter in this request"
}}, status=HTTP_BAD_REQUEST)
if 'anon_hops' in parameters:
anon_hops = int(parameters['anon_hops'])
try:
Expand All @@ -529,7 +546,10 @@ async def update_download(self, request: Request) -> RESTResponse: # noqa: C901
selected_files_list = parameters["selected_files"]
num_files = len(download.tdef.get_files())
if not all(0 <= index < num_files for index in selected_files_list):
return RESTResponse({"error": "index out of range"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "index out of range"
}}, status=HTTP_BAD_REQUEST)
download.set_selected_files(selected_files_list)

if state := parameters.get("state"):
Expand All @@ -542,12 +562,17 @@ async def update_download(self, request: Request) -> RESTResponse: # noqa: C901
elif state == "move_storage":
dest_dir = Path(parameters["dest_dir"])
if not dest_dir.exists():
return RESTResponse({"error": f"Target directory ({dest_dir}) does not exist"},
status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": f"Target directory ({dest_dir}) does not exist"
}}, status=HTTP_BAD_REQUEST)
download.move_storage(dest_dir)
download.checkpoint()
else:
return RESTResponse({"error": "unknown state parameter"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "unknown state parameter"
}}, status=HTTP_BAD_REQUEST)

return RESTResponse({"modified": True, "infohash": hexlify(download.get_def().get_infohash()).decode()})

Expand Down Expand Up @@ -615,13 +640,19 @@ async def add_tracker(self, request: Request) -> RESTResponse:
parameters = await request.json()
url = parameters.get("url")
if not url:
return RESTResponse({"error": "url parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "url parameter missing"
}}, status=HTTP_BAD_REQUEST)

try:
download.add_trackers([url])
download.handle.force_reannounce(0, len(download.handle.trackers()) - 1)
except RuntimeError as e:
return RESTResponse({"error": str(e)}, status=HTTP_INTERNAL_SERVER_ERROR)
return RESTResponse({"error": {
"handled": True,
"message": str(e)
}}, status=HTTP_INTERNAL_SERVER_ERROR)

return RESTResponse({"added": True})

Expand Down Expand Up @@ -657,13 +688,19 @@ async def remove_tracker(self, request: Request) -> RESTResponse:
parameters = await request.json()
url = parameters.get("url")
if not url:
return RESTResponse({"error": "url parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "url parameter missing"
}}, status=HTTP_BAD_REQUEST)

try:
download.handle.replace_trackers([tracker for tracker in download.handle.trackers()
if tracker["url"] != url])
except RuntimeError as e:
return RESTResponse({"error": str(e)}, status=HTTP_INTERNAL_SERVER_ERROR)
return RESTResponse({"error": {
"handled": True,
"message": str(e)
}}, status=HTTP_INTERNAL_SERVER_ERROR)

return RESTResponse({"removed": True})

Expand Down Expand Up @@ -699,15 +736,21 @@ async def tracker_force_announce(self, request: Request) -> RESTResponse:
parameters = await request.json()
url = parameters.get("url")
if not url:
return RESTResponse({"error": "url parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "url parameter missing"
}}, status=HTTP_BAD_REQUEST)

try:
for i, tracker in enumerate(download.handle.trackers()):
if tracker["url"] == url:
download.handle.force_reannounce(0, i)
break
except RuntimeError as e:
return RESTResponse({"error": str(e)}, status=HTTP_INTERNAL_SERVER_ERROR)
return RESTResponse({"error": {
"handled": True,
"message": str(e)
}}, status=HTTP_INTERNAL_SERVER_ERROR)

return RESTResponse({"forced": True})

Expand Down Expand Up @@ -804,7 +847,10 @@ async def collapse_tree_directory(self, request: Request) -> RESTResponse:
params = request.query
path = params.get("path")
if not path:
return RESTResponse({"error": "path parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "path parameter missing"
}}, status=HTTP_BAD_REQUEST)

download.tdef.torrent_file_tree.collapse(Path(path))

Expand Down Expand Up @@ -845,7 +891,10 @@ async def expand_tree_directory(self, request: Request) -> RESTResponse:
params = request.query
path = params.get("path")
if not path:
return RESTResponse({"error": "path parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "path parameter missing"
}}, status=HTTP_BAD_REQUEST)

download.tdef.torrent_file_tree.expand(Path(path))

Expand Down Expand Up @@ -884,7 +933,10 @@ async def select_tree_path(self, request: Request) -> RESTResponse:
params = request.query
path = params.get("path")
if not path:
return RESTResponse({"error": "path parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "path parameter missing"
}}, status=HTTP_BAD_REQUEST)

download.set_selected_file_or_dir(Path(path), True)

Expand Down Expand Up @@ -923,7 +975,10 @@ async def deselect_tree_path(self, request: Request) -> RESTResponse:
params = request.query
path = params.get("path")
if not path:
return RESTResponse({"error": "path parameter missing"}, status=HTTP_BAD_REQUEST)
return RESTResponse({"error": {
"handled": True,
"message": "path parameter missing"
}}, status=HTTP_BAD_REQUEST)

download.set_selected_file_or_dir(Path(path), False)

Expand Down
Loading