Add RESTEndpoint.bad_request() method that logs 4xx error and creates…

… RESTResponse object

src/tribler-core/tribler_core/components/libtorrent/restapi/create_torrent_endpoint.py

@@ -69,7 +69,7 @@ async def create_torrent(self, request):
  if 'files' in parameters and parameters['files']:
  file_path_list = [ensure_unicode(f, 'utf-8') for f in parameters['files']]
  else:
- return RESTResponse({"error": "files parameter missing"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("files parameter missing")
 
  if 'description' in parameters and parameters['description']:
  params['comment'] = parameters['description']

src/tribler-core/tribler_core/components/libtorrent/restapi/downloads_endpoint.py

@@ -17,9 +17,6 @@
 from tribler_core.components.libtorrent.download_manager.stream import STREAM_PAUSE_TIME, StreamChunk
 from tribler_core.components.libtorrent.utils.libtorrent_helper import libtorrent as lt
 from tribler_core.components.restapi.rest.rest_endpoint import (
- HTTP_BAD_REQUEST,
- HTTP_INTERNAL_SERVER_ERROR,
- HTTP_NOT_FOUND,
  RESTEndpoint,
  RESTResponse,
  RESTStreamResponse,
@@ -104,13 +101,6 @@ def setup_routes(self):
  web.get('/{infohash}/files', self.get_files),
  web.get('/{infohash}/stream/{fileindex}', self.stream, allow_head=False)])
 
- @staticmethod
- def return_404(request, message="this download does not exist"):
- """
- Returns a 404 response code if your channel has not been created.
- """
- return RESTResponse({"error": message}, status=HTTP_NOT_FOUND)
-
  @staticmethod
  def create_dconfig_from_params(parameters):
  """
@@ -376,16 +366,16 @@ async def add_download(self, request):
  params = await request.json()
  uri = params.get('uri')
  if not uri:
- return RESTResponse({"error": "uri parameter missing"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("uri parameter missing")
 
  download_config, error = DownloadsEndpoint.create_dconfig_from_params(params)
  if error:
- return RESTResponse({"error": error}, status=HTTP_BAD_REQUEST)
+ return self.bad_request(error)
 
  try:
  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 self.internal_error(e)
 
  return RESTResponse({"started": True, "infohash": hexlify(download.get_def().get_infohash())})
 
@@ -412,12 +402,12 @@ async def add_download(self, request):
  async def delete_download(self, request):
  parameters = await request.json()
  if 'remove_data' not in parameters:
- return RESTResponse({"error": "remove_data parameter missing"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request(f"remove_data parameter missing", parameters=parameters)
 
  infohash = unhexlify(request.match_info['infohash'])
  download = self.download_manager.get_download(infohash)
  if not download:
- return DownloadsEndpoint.return_404(request)
+ return self.not_found("download does not exist")
 
  try:
  await self.download_manager.remove_download(download, remove_content=parameters['remove_data'])
@@ -432,8 +422,7 @@ async def vod_response(self, download, parameters, request, vod_mode):
  if vod_mode:
  file_index = parameters.get("fileindex")
  if file_index is None:
- return RESTResponse({"error": "fileindex is necessary to enable vod_mode"},
- status=HTTP_BAD_REQUEST)
+ return self.bad_request(f"fileindex is necessary to enable vod_mode", parameters=parameters)
  if download.stream is None:
  download.add_stream()
  if not download.stream.enabled or download.stream.fileindex != file_index:
@@ -480,19 +469,17 @@ async def update_download(self, request):
  infohash = unhexlify(request.match_info['infohash'])
  download = self.download_manager.get_download(infohash)
  if not download:
- return DownloadsEndpoint.return_404(request)
+ return self.not_found("download does not exist")
 
  parameters = await request.json()
  vod_mode = parameters.get("vod_mode")
  if vod_mode is not None:
  if not isinstance(vod_mode, bool):
- return RESTResponse({"error": "vod_mode must be bool flag"},
- status=HTTP_BAD_REQUEST)
+ return self.bad_request(f"vod_mode must be bool flag. Got: {vod_mode!r}")
  return await self.vod_response(download, parameters, request, vod_mode)
 
  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 self.bad_request(f"anon_hops must be the only parameter in request", parameters=parameters)
  elif 'anon_hops' in parameters:
  anon_hops = int(parameters['anon_hops'])
  try:
@@ -503,11 +490,13 @@ async def update_download(self, request):
  return RESTResponse({"modified": True, "infohash": hexlify(download.get_def().get_infohash())})
 
  if 'selected_files' in parameters:
- selected_files_list = parameters['selected_files']
+ selected_files = 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)
- download.set_selected_files(selected_files_list)
+ for index in selected_files:
+ if not 0 <= index < num_files:
+ return self.bad_request(f"index of file out of range: {index}", num_files=num_files,
+ selected_files=selected_files)
+ download.set_selected_files(selected_files)
 
  if parameters.get('state'):
  state = parameters['state']
@@ -524,7 +513,7 @@ async def update_download(self, request):
  download.move_storage(dest_dir)
  download.checkpoint()
  else:
- return RESTResponse({"error": "unknown state parameter"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request(f"unknown state parameter: {state!r}")
 
  return RESTResponse({"modified": True, "infohash": hexlify(download.get_def().get_infohash())})
 
@@ -546,11 +535,11 @@ async def get_torrent(self, request):
  infohash = unhexlify(request.match_info['infohash'])
  download = self.download_manager.get_download(infohash)
  if not download:
- return DownloadsEndpoint.return_404(request)
+ return self.not_found("download does not exist")
 
  torrent = download.get_torrent_data()
  if not torrent:
- return DownloadsEndpoint.return_404(request)
+ return self.not_found("download does not exist")
 
  return RESTResponse(lt.bencode(torrent), headers={'content-type': 'application/x-bittorrent',
  'Content-Disposition': 'attachment; filename=%s.torrent'
@@ -580,7 +569,7 @@ async def get_files(self, request):
  infohash = unhexlify(request.match_info['infohash'])
  download = self.download_manager.get_download(infohash)
  if not download:
- return DownloadsEndpoint.return_404(request)
+ return self.not_found("download does not exist")
  return RESTResponse({"files": self.get_files_info_json(download)})
 
  @docs(
@@ -608,7 +597,7 @@ async def stream(self, request):
  infohash = unhexlify(request.match_info['infohash'])
  download = self.download_manager.get_download(infohash)
  if not download:
- return DownloadsEndpoint.return_404(request)
+ return self.not_found("download does not exist")
 
  file_index = int(request.match_info['fileindex'])
 

src/tribler-core/tribler_core/components/libtorrent/restapi/tests/test_downloads_endpoint.py

@@ -379,7 +379,7 @@ async def test_download_unknown_state(mock_dlmgr, test_download, rest_api):
 
  await do_request(rest_api, f'downloads/{test_download.infohash}', expected_code=400,
  post_data={"state": "abc"}, request_type='PATCH',
- expected_json={"error": "unknown state parameter"})
+ expected_json={"error": "unknown state parameter: 'abc'", "context": {}})
 
 
 async def test_change_hops_error(mock_dlmgr, test_download, rest_api):

src/tribler-core/tribler_core/components/libtorrent/restapi/torrentinfo_endpoint.py

@@ -15,8 +15,6 @@
 from tribler_core.components.libtorrent.utils.libtorrent_helper import libtorrent as lt
 from tribler_core.components.metadata_store.db.orm_bindings.torrent_metadata import tdef_to_metadata_dict
 from tribler_core.components.restapi.rest.rest_endpoint import (
- HTTP_BAD_REQUEST,
- HTTP_INTERNAL_SERVER_ERROR,
  RESTEndpoint,
  RESTResponse,
 )
@@ -82,10 +80,10 @@ async def get_torrent_info(self, request):
  try:
  hops = int(hops)
  except ValueError:
- return RESTResponse({"error": f"wrong value of 'hops' parameter: {hops}"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request(f"wrong value of 'hops' parameter: {hops}")
 
  if not uri:
- return RESTResponse({"error": "uri parameter missing"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("uri parameter missing")
 
  metainfo = None
  scheme = scheme_from_uri(uri)
@@ -95,14 +93,13 @@ async def get_torrent_info(self, request):
  try:
  tdef = TorrentDef.load(file)
  metainfo = tdef.metainfo
- except (TypeError, RuntimeError):
- return RESTResponse({"error": f"error while decoding torrent file: {file}"},
- status=HTTP_INTERNAL_SERVER_ERROR)
+ except (TypeError, RuntimeError) as e:
+ return self.internal_error(e, f"error while decoding torrent file: {file}")
  elif scheme in (HTTP_SCHEME, HTTPS_SCHEME):
  try:
  response = await query_http_uri(uri)
  except (ServerConnectionError, ClientResponseError) as e:
- return RESTResponse({"error": str(e)}, status=HTTP_INTERNAL_SERVER_ERROR)
+ return self.internal_error(e)
 
  if response.startswith(b'magnet'):
  _, infohash, _ = parse_magnetlink(response)
@@ -113,17 +110,17 @@ async def get_torrent_info(self, request):
  elif scheme == MAGNET_SCHEME:
  infohash = parse_magnetlink(uri)[1]
  if infohash is None:
- return RESTResponse({"error": "missing infohash"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("missing infohash")
  metainfo = await self.download_manager.get_metainfo(infohash, timeout=60, hops=hops, url=uri)
  else:
- return RESTResponse({"error": "invalid uri"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("invalid uri")
 
  if not metainfo:
- return RESTResponse({"error": "metainfo error"}, status=HTTP_INTERNAL_SERVER_ERROR)
+ return self.internal_error(msg="metainfo error")
 
  if not isinstance(metainfo, dict) or b'info' not in metainfo:
  self._logger.warning("Received metainfo is not a valid dictionary")
- return RESTResponse({"error": "invalid response"}, status=HTTP_INTERNAL_SERVER_ERROR)
+ return self.internal_error(msg="Received metainfo is not a valid dictionary")
 
  # Add the torrent to GigaChannel as a free-for-all entry, so others can search it
  self.download_manager.notifier.notify(

src/tribler-core/tribler_core/components/metadata_store/restapi/channels_endpoint.py

@@ -285,16 +285,17 @@ async def copy_channel(self, request):
  try:
  request_parsed = await request.json()
  except (ContentTypeError, ValueError):
- return RESTResponse({"error": "Bad JSON"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("Bad JSON")
 
  if not target_collection and not personal_root:
- return RESTResponse({"error": "Target channel not found"}, status=HTTP_NOT_FOUND)
+ return self.not_found("Target channel not found")
+
  results_list = []
  for entry in request_parsed:
  public_key, id_ = unhexlify(entry["public_key"]), entry["id"]
  source = self.mds.ChannelNode.get(public_key=public_key, id_=id_)
  if not source:
- return RESTResponse({"error": "Source entry not found"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("Source entry not found")
  # We must upgrade Collections to Channels when moving them to root channel, and, vice-versa,
  # downgrade Channels to Collections when moving them into existing channels
  if isinstance(source, self.mds.CollectionNode):
@@ -379,7 +380,7 @@ async def add_torrent_to_channel(self, request):
  with db_session:
  channel = self.mds.CollectionNode.get(public_key=channel_pk, id_=channel_id)
  if not channel:
- return RESTResponse({"error": "Unknown channel"}, status=HTTP_NOT_FOUND)
+ return self.not_found("Unknown channel")
 
  parameters = await request.json()
 
@@ -407,7 +408,7 @@ async def add_torrent_to_channel(self, request):
  raise RuntimeError("Metainfo timeout")
  tdef = TorrentDef.load_from_dict(meta_info)
  else:
- return RESTResponse({"error": "unknown uri type"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("unknown uri type")
 
  added = 0
  if tdef:
@@ -419,23 +420,21 @@ async def add_torrent_to_channel(self, request):
  if parameters.get('torrents_dir', None):
  torrents_dir = parameters['torrents_dir']
  if not Path(torrents_dir).is_absolute():
- return RESTResponse({"error": "the torrents_dir should point to a directory"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("the torrents_dir should point to a directory")
 
  recursive = False
  if parameters.get('recursive'):
  recursive = parameters['recursive']
  if not torrents_dir:
- return RESTResponse(
- {"error": "the torrents_dir parameter should be provided when the recursive parameter is set"},
- status=HTTP_BAD_REQUEST,
- )
+ return self.bad_request(
+ "the torrents_dir parameter should be provided when the recursive parameter is set")
 
  if torrents_dir:
  torrents_list, errors_list = channel.add_torrents_from_dir(torrents_dir, recursive)
  return RESTResponse({"added": len(torrents_list), "errors": errors_list})
 
  if not parameters.get('torrent', None):
- return RESTResponse({"error": "torrent parameter missing"}, status=HTTP_BAD_REQUEST)
+ return self.bad_request("torrent parameter missing")
 
  # Try to parse the torrent data
  # Any errors will be handled by the error_middleware