diff --git a/app/src/api.js b/app/src/api.js index b76bdef3..98b0e428 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -643,7 +643,7 @@ export async function deleteFile(paths) { toast({ type: 'error', title: 'Delete error', - description: content['message'] || content['summary'], + description: content['message'], time: 5000, }); } @@ -886,7 +886,7 @@ export async function deleteTag(id, name) { const content = await response.json(); if (content['code'] === 'USED_TAG') { toast({ - type: 'error', title: 'Error!', description: content['error'] || content['summary'], time: 5000, + type: 'error', title: 'Error!', description: content['message'], time: 5000, }) } } else if (response.status !== 204) { diff --git a/modules/archive/errors.py b/modules/archive/errors.py index c463cdac..3551190d 100644 --- a/modules/archive/errors.py +++ b/modules/archive/errors.py @@ -5,5 +5,5 @@ from wrolpi.errors import APIError class InvalidArchive(APIError): code = 'INVALID_ARCHIVE' - summary = 'The archive is invalid. See server logs.' - status = HTTPStatus.BAD_REQUEST + message = 'The archive is invalid. See server logs.' + status_code = HTTPStatus.BAD_REQUEST diff --git a/modules/inventory/errors.py b/modules/inventory/errors.py index 8e14291d..3506d381 100644 --- a/modules/inventory/errors.py +++ b/modules/inventory/errors.py @@ -5,11 +5,11 @@ from wrolpi.errors import APIError class NoInventories(APIError): code = 'NO_INVENTORIES' - summary = 'No Inventories' - status = HTTPStatus.BAD_REQUEST + message = 'No Inventories' + status_code = HTTPStatus.BAD_REQUEST class InventoriesVersionMismatch(APIError): code = 'INVENTORIES_VERSION_MISMATCH' - summary = 'Inventories version in the DB does not match the inventories config' - status = HTTPStatus.BAD_REQUEST + message = 'Inventories version in the DB does not match the inventories config' + status_code = HTTPStatus.BAD_REQUEST diff --git a/modules/map/test/test_api.py b/modules/map/test/test_api.py index 547cf395..455b51bc 100644 --- a/modules/map/test/test_api.py +++ b/modules/map/test/test_api.py @@ -135,4 +135,4 @@ def test_empty_import(test_client, test_session, test_directory): body = {'files': []} request, response = test_client.post('/api/map/import', content=json.dumps(body)) assert response.status_code == HTTPStatus.BAD_REQUEST - assert 'validate' in response.json['summary'] + assert 'validate' in response.json['message'] diff --git a/modules/otp/errors.py b/modules/otp/errors.py index f7c73840..a27a2cd5 100644 --- a/modules/otp/errors.py +++ b/modules/otp/errors.py @@ -5,17 +5,17 @@ from wrolpi.errors import APIError class InvalidOTP(APIError): code = 'INVALID_OTP' - summary = 'OTP has invalid characters' - status = HTTPStatus.BAD_REQUEST + message = 'OTP has invalid characters' + status_code = HTTPStatus.BAD_REQUEST class InvalidPlaintext(APIError): code = 'INVALID_PLAINTEXT' - summary = 'Plaintext has invalid characters' - status = HTTPStatus.BAD_REQUEST + message = 'Plaintext has invalid characters' + status_code = HTTPStatus.BAD_REQUEST class InvalidCiphertext(APIError): code = 'INVALID_CIPHERTEXT' - summary = 'Ciphertext has invalid characters' - status = HTTPStatus.BAD_REQUEST + message = 'Ciphertext has invalid characters' + status_code = HTTPStatus.BAD_REQUEST diff --git a/modules/videos/channel/test/test_api.py b/modules/videos/channel/test/test_api.py index e7df28e3..3589f4ec 100644 --- a/modules/videos/channel/test/test_api.py +++ b/modules/videos/channel/test/test_api.py @@ -40,7 +40,7 @@ def test_get_video(test_client, test_session, simple_channel, video_factory): assert response.status_code == HTTPStatus.NOT_FOUND, response.json assert response.json == {'code': 'UNKNOWN_VIDEO', 'error': 'Cannot find Video with id 10', - 'summary': 'The video could not be found.'} + 'message': 'The video could not be found.'} # Get the video info we inserted _, response = test_client.get('/api/videos/video/1') @@ -207,11 +207,11 @@ def test_channel_conflicts(test_client, test_session, test_directory): request, response = _post_channel(new_channel) assert response.status_code == HTTPStatus.BAD_REQUEST assert response.json == {'cause': {'code': 'CHANNEL_NAME_CONFLICT', - 'error': '', - 'summary': 'The channel name is already taken.'}, + 'error': 'The channel name is already taken.', + 'message': 'The channel name is already taken.'}, 'code': 'VALIDATION_ERROR', - 'error': '', - 'summary': 'Could not validate the contents of the request'} + 'error': 'Could not validate the contents of the request', + 'message': 'Could not validate the contents of the request'} # Directory was already used new_channel = dict( @@ -221,11 +221,11 @@ def test_channel_conflicts(test_client, test_session, test_directory): request, response = _post_channel(new_channel) assert response.status_code == HTTPStatus.BAD_REQUEST assert response.json == {'cause': {'code': 'CHANNEL_DIRECTORY_CONFLICT', - 'error': '', - 'summary': 'The directory is already used by another channel.'}, + 'error': 'The directory is already used by another channel.', + 'message': 'The directory is already used by another channel.'}, 'code': 'VALIDATION_ERROR', - 'error': '', - 'summary': 'Could not validate the contents of the request'} + 'error': 'Could not validate the contents of the request', + 'message': 'Could not validate the contents of the request'} # URL is already used channel_directory3 = tempfile.TemporaryDirectory(dir=test_directory).name @@ -238,11 +238,11 @@ def test_channel_conflicts(test_client, test_session, test_directory): request, response = _post_channel(new_channel) assert response.status_code == HTTPStatus.BAD_REQUEST assert response.json == {'cause': {'code': 'CHANNEL_URL_CONFLICT', - 'error': '', - 'summary': 'The URL is already used by another channel.'}, + 'error': 'The URL is already used by another channel.', + 'message': 'The URL is already used by another channel.'}, 'code': 'VALIDATION_ERROR', - 'error': '', - 'summary': 'Could not validate the contents of the request'} + 'error': 'Could not validate the contents of the request', + 'message': 'Could not validate the contents of the request'} def test_channel_empty_url_doesnt_conflict(test_client, test_session, test_directory): diff --git a/modules/videos/channel/test/test_models.py b/modules/videos/channel/test/test_models.py index 958f81c0..3917cc7a 100644 --- a/modules/videos/channel/test/test_models.py +++ b/modules/videos/channel/test/test_models.py @@ -27,7 +27,7 @@ async def test_delete_channel_with_videos(test_session, test_async_client, chann # Delete the Channel. request, response = await test_async_client.delete(f'/api/videos/channels/{channel.id}') - assert response.status == HTTPStatus.NO_CONTENT + assert response.status_code == HTTPStatus.NO_CONTENT # Video still exists, but has no channel. video: Video = test_session.query(Video).one() @@ -53,9 +53,9 @@ async def test_nested_channel_directories(test_session, test_async_client, test_ directory=str(channel2_directory), ) request, response = await test_async_client.post('/api/videos/channels', content=json.dumps(content)) - assert response.status == HTTPStatus.BAD_REQUEST + assert response.status_code == HTTPStatus.BAD_REQUEST assert response.json['cause'] and \ - response.json['cause']['summary'] == 'The directory is already used by another channel.' + response.json['cause']['message'] == 'The directory is already used by another channel.' # Channel 3 cannot be above Channel 1's directory. content = dict( @@ -63,6 +63,6 @@ async def test_nested_channel_directories(test_session, test_async_client, test_ directory=str(test_directory), ) request, response = await test_async_client.post('/api/videos/channels', content=json.dumps(content)) - assert response.status == HTTPStatus.BAD_REQUEST + assert response.status_code == HTTPStatus.BAD_REQUEST assert response.json['cause'] and \ - response.json['cause']['summary'] == 'The directory is already used by another channel.' + response.json['cause']['message'] == 'The directory is already used by another channel.' diff --git a/modules/videos/errors.py b/modules/videos/errors.py index d3140d46..18633148 100644 --- a/modules/videos/errors.py +++ b/modules/videos/errors.py @@ -5,41 +5,41 @@ from wrolpi.errors import APIError class UnknownVideo(APIError): code = 'UNKNOWN_VIDEO' - summary = 'The video could not be found.' - status = HTTPStatus.NOT_FOUND + message = 'The video could not be found.' + status_code = HTTPStatus.NOT_FOUND class UnknownChannel(APIError): code = 'UNKNOWN_CHANNEL' - summary = 'The channel could not be found.' - status = HTTPStatus.NOT_FOUND + message = 'The channel could not be found.' + status_code = HTTPStatus.NOT_FOUND class ChannelNameConflict(APIError): code = 'CHANNEL_NAME_CONFLICT' - summary = 'The channel name is already taken.' - status = HTTPStatus.BAD_REQUEST + message = 'The channel name is already taken.' + status_code = HTTPStatus.BAD_REQUEST class ChannelURLConflict(APIError): code = 'CHANNEL_URL_CONFLICT' - summary = 'The URL is already used by another channel.' - status = HTTPStatus.BAD_REQUEST + message = 'The URL is already used by another channel.' + status_code = HTTPStatus.BAD_REQUEST class ChannelDirectoryConflict(APIError): code = 'CHANNEL_DIRECTORY_CONFLICT' - summary = 'The directory is already used by another channel.' - status = HTTPStatus.BAD_REQUEST + message = 'The directory is already used by another channel.' + status_code = HTTPStatus.BAD_REQUEST class ChannelSourceIdConflict(APIError): code = 'CHANNEL_SOURCE_ID_CONFLICT' - summary = 'Search is empty, search_str must have content.' - status = HTTPStatus.BAD_REQUEST + message = 'Search is empty, search_str must have content.' + status_code = HTTPStatus.BAD_REQUEST class ChannelURLEmpty(APIError): code = 'CHANNEL_URL_EMPTY' - summary = 'This channel has no URL' - status = HTTPStatus.BAD_REQUEST + message = 'This channel has no URL' + status_code = HTTPStatus.BAD_REQUEST diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index f17e8068..fdde4302 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -154,7 +154,7 @@ async def test_download_channel(test_session, simple_channel, video_download_man downloads = filter(lambda i: 'watch' in i.url, downloads) assert {i.url for i in downloads} == \ {'https://youtube.com/watch?v=video_2_url', 'https://youtube.com/watch?v=video_1_url'} - assert all(i.status == 'complete' for i in downloads) + assert all(i.status_code == 'complete' for i in downloads) # A channel with `match_regex` only returns matching video URLs. simple_channel.match_regex = '.*(2).*' diff --git a/modules/zim/errors.py b/modules/zim/errors.py index 64273d51..6afa149f 100644 --- a/modules/zim/errors.py +++ b/modules/zim/errors.py @@ -5,23 +5,23 @@ from wrolpi.errors import APIError class UnknownZim(APIError): code = 'UNKNOWN_ZIM' - summary = 'Failed to find the Zim' - status = HTTPStatus.NOT_FOUND + message = 'Failed to find the Zim' + status_code = HTTPStatus.NOT_FOUND class UnknownZimEntry(APIError): code = 'UNKNOWN_ZIM_ENTRY' - summary = 'Failed to find the Zim entry at that path' - status = HTTPStatus.NOT_FOUND + message = 'Failed to find the Zim entry at that path' + status_code = HTTPStatus.NOT_FOUND class UnknownZimTagEntry(APIError): code = 'UNKNOWN_ZIM_TAG_ENTRY' - summary = 'Failed to find the TagZimEntry' - status = HTTPStatus.NOT_FOUND + message = 'Failed to find the TagZimEntry' + status_code = HTTPStatus.NOT_FOUND class UnknownZimSubscription(APIError): code = 'UNKNOWN_ZIM_SUBSCRIPTION' - summary = 'Failed to find the ZimSubscription' - status = HTTPStatus.NOT_FOUND + message = 'Failed to find the ZimSubscription' + status_code = HTTPStatus.NOT_FOUND diff --git a/wrolpi/api_utils.py b/wrolpi/api_utils.py index de998765..be0da6cb 100644 --- a/wrolpi/api_utils.py +++ b/wrolpi/api_utils.py @@ -2,12 +2,14 @@ import json from datetime import datetime, timezone, date from decimal import Decimal from functools import wraps +from http import HTTPStatus from pathlib import Path -from sanic import response, HTTPResponse, Request, Sanic +from sanic import response, HTTPResponse, Request, Sanic, SanicException from wrolpi.common import Base, get_media_directory, logger, LOGGING_CONFIG from wrolpi.errors import APIError +from wrolpi.vars import PYTEST logger = logger.getChild(__name__) @@ -68,14 +70,17 @@ class CustomJSONEncoder(json.JSONEncoder): def get_error_json(exception: BaseException): """Return a JSON representation of the Exception instance.""" if isinstance(exception, APIError): - # Error especially defined for WROLPi. - body = dict(error=str(exception), summary=exception.summary, code=exception.code) + # An exception from WROLPi. + body = dict(error=str(exception), message=exception.message, code=exception.code) + elif isinstance(exception, SanicException): + # An exception from Sanic. + body = dict(error=str(exception), message=exception.message, code=type(exception).__name__) else: # Not a WROLPi APIError error. body = dict( error=str(exception), - summary=None, - code=None, + message=None, + code=type(exception).__name__, ) if exception.__cause__: # This exception was caused by another, follow the stack. @@ -83,10 +88,23 @@ def get_error_json(exception: BaseException): return body -def json_error_handler(request: Request, exception: APIError): - body = get_error_json(exception) +def json_error_handler(request: Request, exception: Exception): + """Converts all API APIError/SanicException to more informative json object.""" + try: + body = get_error_json(exception) + except Exception as e: + logger.error('Failed to create error json', exc_info=e) + raise + error = repr(str(body["error"])) - summary = repr(str(body["summary"])) + message = repr(str(body["message"])) code = body['code'] - logger.debug(f'API returning JSON error {exception=} {error=} {summary=} {code=}') - return json_response(body, exception.status) + logger.error(f'API returning JSON error {type(exception).__name__} {error=} {message=} {code=}') + if isinstance(exception, SanicException): + return json_response(body, exception.status_code) + + if PYTEST: + logger.error('Unexpected error', exc_info=exception) + + # Some unknown error, use internal error code. + return json_response(body, HTTPStatus.INTERNAL_SERVER_ERROR) diff --git a/wrolpi/common.py b/wrolpi/common.py index 983b90ff..132d4836 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -985,21 +985,21 @@ async def aiohttp_post(url: str, json_, timeout: int = None) -> Tuple[Dict, int] """Perform an async aiohttp POST request. Return the json contents.""" async with aiohttp_session(timeout) as session: async with session.post(url, json=json_) as response: - return await response.json(), response.status + return await response.json(), response.status_code async def aiohttp_get(url: str, timeout: int = None, headers: dict = None) -> Tuple[bytes, int]: """Perform an async aiohttp GET request. Return the contents.""" async with aiohttp_session(timeout) as session: async with session.get(url, headers=headers) as response: - return await response.content.read(), response.status + return await response.content.read(), response.status_code async def aiohttp_head(url: str, timeout: int = None) -> Tuple[ClientResponse, int]: """Perform an async aiohttp HEAD request. Return the contents.""" async with aiohttp_session(timeout) as session: async with session.head(url) as response: - return response, response.status + return response, response.status_code async def speed_test(url: str) -> int: diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index b7ea6617..8078d563 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -949,7 +949,7 @@ class DownloadManager: @optional_session def delete_once(self, session: Session): - """Delete any once-download records.""" + """Delete all once-download records.""" stmt = self._delete_downloads_q(once=True) deleted_ids = [i for i, in session.execute(stmt).fetchall()] session.commit() diff --git a/wrolpi/errors.py b/wrolpi/errors.py index 92619e65..cb33824a 100644 --- a/wrolpi/errors.py +++ b/wrolpi/errors.py @@ -1,157 +1,159 @@ from http import HTTPStatus +from sanic import SanicException -class APIError(Exception): + +class APIError(SanicException): code = 'APIError' - summary = 'No summary defined' - status = HTTPStatus.INTERNAL_SERVER_ERROR + message = 'No summary defined' + status_code = HTTPStatus.INTERNAL_SERVER_ERROR class UnknownDirectory(APIError): code = 'UNKNOWN_DIRECTORY' - summary = 'The directory does not exist.' - status = HTTPStatus.NOT_FOUND + message = 'The directory does not exist.' + status_code = HTTPStatus.NOT_FOUND class UnknownFile(APIError): code = 'UNKNOWN_FILE' - summary = 'The file does not exist.' - status = HTTPStatus.NOT_FOUND + message = 'The file does not exist.' + status_code = HTTPStatus.NOT_FOUND class SearchEmpty(APIError): code = 'SEARCH_EMPTY' - summary = 'Search is empty, search_str must have content.' - status = HTTPStatus.BAD_REQUEST + message = 'Search is empty, search_str must have content.' + status_code = HTTPStatus.BAD_REQUEST class ValidationError(APIError): code = 'VALIDATION_ERROR' - summary = 'Could not validate the contents of the request' - status = HTTPStatus.BAD_REQUEST + message = 'Could not validate the contents of the request' + status_code = HTTPStatus.BAD_REQUEST class InvalidOrderBy(ValidationError): code = 'INVALID_ORDER_BY' - summary = 'Invalid order_by' - status = HTTPStatus.BAD_REQUEST + message = 'Invalid order_by' + status_code = HTTPStatus.BAD_REQUEST class WROLModeEnabled(APIError): code = 'WROL_MODE_ENABLED' - summary = 'This method is disabled while WROL Mode is enabled.' - status = HTTPStatus.FORBIDDEN + message = 'This method is disabled while WROL Mode is enabled.' + status_code = HTTPStatus.FORBIDDEN class InvalidDownload(APIError): code = 'INVALID_DOWNLOAD' - summary = 'The URL cannot be downloaded.' - status = HTTPStatus.BAD_REQUEST + message = 'The URL cannot be downloaded.' + status_code = HTTPStatus.BAD_REQUEST class UnrecoverableDownloadError(APIError): code = 'UNRECOVERABLE_DOWNLOAD_ERROR' - summary = 'Download experienced an error which cannot be recovered. Download will be deleted.' - status = HTTPStatus.BAD_REQUEST + message = 'Download experienced an error which cannot be recovered. Download will be deleted.' + status_code = HTTPStatus.BAD_REQUEST class InvalidFile(APIError): code = 'INVALID_FILE' - summary = 'File does not exist or is a directory' - status = HTTPStatus.BAD_REQUEST + message = 'File does not exist or is a directory' + status_code = HTTPStatus.BAD_REQUEST class NativeOnly(APIError): code = 'NATIVE_ONLY' - summary = 'This functionality is only supported outside a docker container' - status = HTTPStatus.BAD_REQUEST + message = 'This functionality is only supported outside a docker container' + status_code = HTTPStatus.BAD_REQUEST class HotspotError(APIError): code = 'HOTSPOT_ERROR' - summary = 'Updating/accessing Hotspot encountered an error' - status = HTTPStatus.INTERNAL_SERVER_ERROR + message = 'Updating/accessing Hotspot encountered an error' + status_code = HTTPStatus.INTERNAL_SERVER_ERROR class LogLevelError(APIError): code = 'LOG_LEVEL_ERROR' - summary = 'Log level is invalid' - status = HTTPStatus.BAD_REQUEST + message = 'Log level is invalid' + status_code = HTTPStatus.BAD_REQUEST class UnknownArchive(APIError): code = 'UNKNOWN_ARCHIVE' - summary = 'Unable to find the archive' - status = HTTPStatus.NOT_FOUND + message = 'Unable to find the archive' + status_code = HTTPStatus.NOT_FOUND class UnknownTag(APIError): code = 'UNKNOWN_TAG' - summary = 'Unable to find the tag' - status = HTTPStatus.NOT_FOUND + message = 'Unable to find the tag' + status_code = HTTPStatus.NOT_FOUND class UsedTag(APIError): code = 'USED_TAG' - summary = 'Tag is being used' - status = HTTPStatus.BAD_REQUEST + message = 'Tag is being used' + status_code = HTTPStatus.BAD_REQUEST class InvalidTag(APIError): code = 'INVALID_TAG' - summary = 'Tag is not allowed' - status = HTTPStatus.BAD_REQUEST + message = 'Tag is not allowed' + status_code = HTTPStatus.BAD_REQUEST class FileGroupIsTagged(APIError): code = 'FILE_GROUP_IS_TAGGED' - summary = 'File group is tagged' - status = HTTPStatus.CONFLICT + message = 'File group is tagged' + status_code = HTTPStatus.CONFLICT class FileUploadFailed(APIError): code = 'FILE_UPLOAD_FAILED' - summary = 'Failed to upload file' - status = HTTPStatus.BAD_REQUEST + message = 'Failed to upload file' + status_code = HTTPStatus.BAD_REQUEST class FileConflict(APIError): code = 'FILE_CONFLICT' - summary = 'File/directory already exists' - status = HTTPStatus.CONFLICT + message = 'File/directory already exists' + status_code = HTTPStatus.CONFLICT class HotspotPasswordTooShort(APIError): code = 'HOTSPOT_PASSWORD_TOO_SHORT' - summary = 'Hotspot password must be at least 8 characters' - status = HTTPStatus.BAD_REQUEST + message = 'Hotspot password must be at least 8 characters' + status_code = HTTPStatus.BAD_REQUEST class ShutdownFailed(APIError): code = 'SHUTDOWN_FAILED' - summary = 'Unable to shutdown the WROLPi' - status = HTTPStatus.INTERNAL_SERVER_ERROR + message = 'Unable to shutdown the WROLPi' + status_code = HTTPStatus.INTERNAL_SERVER_ERROR class InvalidConfig(APIError): code = 'INVALID_CONFIG' - summary = 'Config is invalid' - status = HTTPStatus.BAD_REQUEST + message = 'Config is invalid' + status_code = HTTPStatus.BAD_REQUEST class NoPrimaryFile(APIError): code = 'NO_PRIMARY_FILE' - summary = 'Could not find a primary file in the provided group' - status = HTTPStatus.INTERNAL_SERVER_ERROR + message = 'Could not find a primary file in the provided group' + status_code = HTTPStatus.INTERNAL_SERVER_ERROR class InvalidDatetime(APIError): code = 'INVALID_DATETIME' - summary = 'Unable to parse a datetime string' - status = HTTPStatus.INTERNAL_SERVER_ERROR + message = 'Unable to parse a datetime string' + status_code = HTTPStatus.INTERNAL_SERVER_ERROR class InvalidDirectory(APIError): code = 'INVALID_DIRECTORY' - summary = 'Directory is not valid or impossible' - status = HTTPStatus.BAD_REQUEST + message = 'Directory is not valid or impossible' + status_code = HTTPStatus.BAD_REQUEST diff --git a/wrolpi/files/test/test_ebooks.py b/wrolpi/files/test/test_ebooks.py index 0b6cfeea..5933e141 100644 --- a/wrolpi/files/test/test_ebooks.py +++ b/wrolpi/files/test/test_ebooks.py @@ -80,7 +80,7 @@ async def test_extract_cover(test_session, test_directory, example_epub): def test_search(test_session, test_client, example_epub): """Ebooks are handled in File search results.""" request, response = test_client.post('/api/files/refresh') - assert response.status == HTTPStatus.NO_CONTENT + assert response.status_code == HTTPStatus.NO_CONTENT assert test_session.query(EBook).count() == 1 @@ -92,7 +92,7 @@ def test_search(test_session, test_client, example_epub): content = dict(mimetypes=['application/epub', 'application/x-mobipocket-ebook']) request, response = test_client.post('/api/files/search', content=json.dumps(content)) - assert response.status == HTTPStatus.OK + assert response.status_code == HTTPStatus.OK assert response.json file_group = response.json['file_groups'][0] epub_file = file_group['files'][0] @@ -106,7 +106,7 @@ def test_search(test_session, test_client, example_epub): # No Mobi ebook. content = dict(mimetypes=['application/x-mobipocket-ebook']) request, response = test_client.post('/api/files/search', content=json.dumps(content)) - assert response.status == HTTPStatus.OK + assert response.status_code == HTTPStatus.OK assert response.json assert len(response.json['file_groups']) == 0 diff --git a/wrolpi/root_api.py b/wrolpi/root_api.py index b480aa9e..af91838d 100644 --- a/wrolpi/root_api.py +++ b/wrolpi/root_api.py @@ -25,6 +25,7 @@ from wrolpi.common import logger, get_wrolpi_config, wrol_mode_enabled, get_medi wrol_mode_check, native_only, disable_wrol_mode, enable_wrol_mode, get_global_statistics, url_strip_host, \ set_global_log_level, get_relative_to_media_directory from wrolpi.dates import now +from wrolpi.downloader import download_manager from wrolpi.errors import WROLModeEnabled, APIError, HotspotError, InvalidDownload, \ HotspotPasswordTooShort, NativeOnly, InvalidConfig from wrolpi.events import get_events, Events @@ -131,15 +132,14 @@ async def echo(request: Request): @openapi.description('Get WROLPi settings') @openapi.response(HTTPStatus.OK, schema.SettingsResponse) def get_settings(_: Request): - from wrolpi.downloader import download_manager config = get_wrolpi_config() ignored_directories = [get_relative_to_media_directory(i) for i in config.ignored_directories] settings = { 'archive_directory': config.archive_directory, - 'download_manager_disabled': download_manager.disabled.is_set(), - 'download_manager_stopped': download_manager.stopped.is_set(), + 'download_manager_disabled': download_manager.is_disabled, + 'download_manager_stopped': download_manager.is_stopped, 'download_on_startup': config.download_on_startup, 'download_timeout': config.download_timeout, 'hotspot_device': config.hotspot_device, @@ -249,7 +249,6 @@ def valid_regex(_: Request, body: schema.RegexRequest): @validate(schema.DownloadRequest) @wrol_mode_check async def post_download(_: Request, body: schema.DownloadRequest): - from wrolpi.downloader import download_manager downloader = download_manager.get_downloader_by_name(body.downloader) if not downloader: raise InvalidDownload(f'Cannot find downloader with name {body.downloader}') @@ -283,7 +282,6 @@ async def post_download(_: Request, body: schema.DownloadRequest): @api_bp.post('/download//restart') @openapi.description('Restart a download.') async def restart_download(_: Request, download_id: int): - from wrolpi.downloader import download_manager download_manager.restart_download(download_id) return response.empty() @@ -291,7 +289,6 @@ async def restart_download(_: Request, download_id: int): @api_bp.get('/download') @openapi.description('Get all Downloads that need to be processed.') async def get_downloads(_: Request): - from wrolpi.downloader import download_manager data = download_manager.get_fe_downloads() return json_response(data) @@ -299,7 +296,6 @@ async def get_downloads(_: Request): @api_bp.post('/download//kill') @openapi.description('Kill a download. It will be stopped if it is pending.') async def kill_download(_: Request, download_id: int): - from wrolpi.downloader import download_manager download_manager.kill_download(download_id) return response.empty() @@ -307,7 +303,6 @@ async def kill_download(_: Request, download_id: int): @api_bp.post('/download/kill') @openapi.description('Kill all downloads. Disable downloading.') async def kill_downloads(_: Request): - from wrolpi.downloader import download_manager logger.warning('Disabled downloads') download_manager.disable() return response.empty() @@ -316,7 +311,6 @@ async def kill_downloads(_: Request): @api_bp.post('/download/enable') @openapi.description('Enable and start downloading.') async def enable_downloads(_: Request): - from wrolpi.downloader import download_manager await download_manager.enable() return response.empty() @@ -324,7 +318,6 @@ async def enable_downloads(_: Request): @api_bp.post('/download/clear_completed') @openapi.description('Clear completed downloads') async def clear_completed(_: Request): - from wrolpi.downloader import download_manager download_manager.delete_completed() return response.empty() @@ -332,7 +325,6 @@ async def clear_completed(_: Request): @api_bp.post('/download/clear_failed') @openapi.description('Clear failed downloads') async def clear_failed(_: Request): - from wrolpi.downloader import download_manager download_manager.delete_failed() return response.empty() @@ -340,7 +332,6 @@ async def clear_failed(_: Request): @api_bp.post('/download/delete_once') @openapi.description('Delete all once downloads') async def delete_once(_: Request): - from wrolpi.downloader import download_manager download_manager.delete_once() return response.empty() @@ -349,7 +340,6 @@ async def delete_once(_: Request): @openapi.description('Delete a download') @wrol_mode_check async def delete_download(_: Request, download_id: int): - from wrolpi.downloader import download_manager deleted = download_manager.delete_download(download_id) return response.empty(HTTPStatus.NO_CONTENT if deleted else HTTPStatus.NOT_FOUND) @@ -357,7 +347,6 @@ async def delete_download(_: Request, download_id: int): @api_bp.get('/downloaders') @openapi.description('List all Downloaders that can be specified by the user.') async def get_downloaders(_: Request): - from wrolpi.downloader import download_manager downloaders = download_manager.list_downloaders() disabled = download_manager.disabled.is_set() ret = dict(downloaders=downloaders, manager_disabled=disabled) @@ -407,8 +396,6 @@ async def throttle_off(_: Request): @api_bp.get('/status') @openapi.description('Get the status of CPU/load/etc.') async def get_status(_: Request): - from wrolpi.downloader import download_manager - s = await status.get_status() downloads = dict() if flags.db_up.is_set(): @@ -591,4 +578,4 @@ async def post_search_file_estimates(_: Request, body: schema.SearchFileEstimate return json_response(ret) -api_app.error_handler.add(APIError, json_error_handler) +api_app.error_handler.add(Exception, json_error_handler) diff --git a/wrolpi/scrape_downloader.py b/wrolpi/scrape_downloader.py index 36c77b4e..1ba2a0ea 100644 --- a/wrolpi/scrape_downloader.py +++ b/wrolpi/scrape_downloader.py @@ -42,8 +42,8 @@ class ScrapeHTMLDownloader(Downloader): async def fetch_http(url: str) -> str: async with aiohttp_session(timeout=60 * 5) as session: async with session.get(url, headers=DEFAULT_HTTP_HEADERS) as response: - logger.debug(f'Got status={response.status} from {url}') - if response.status == HTTPStatus.OK: + logger.debug(f'Got status={response.status_code} from {url}') + if response.status_code == HTTPStatus.OK: return await response.text() async def do_download(self, download: Download) -> DownloadResult: diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index f3349f12..25e8304b 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -195,7 +195,7 @@ async def test_recurring_downloads(test_session, test_download_manager, fake_now assert len(downloads) == 1 download = downloads[0] expected = datetime(2020, 1, 1, 1, 0, 0, tzinfo=pytz.UTC) - assert download.is_complete, download.status + assert download.is_complete, download.status_code assert download.next_download == expected assert download.last_successful_download == now_ @@ -210,7 +210,7 @@ async def test_recurring_downloads(test_session, test_download_manager, fake_now test_download_manager.renew_recurring_downloads(test_session) (download,) = list(test_download_manager.get_new_downloads(test_session)) # Download is "new" but has not been downloaded a second time. - assert download.is_new, download.status + assert download.is_new, download.status_code assert download.next_download == expected assert download.last_successful_download == now_ @@ -221,7 +221,7 @@ async def test_recurring_downloads(test_session, test_download_manager, fake_now test_downloader.do_download.assert_called_once() download = test_session.query(Download).one() # Download is deferred, last successful download remains the same. - assert download.is_deferred, download.status + assert download.is_deferred, download.status_code assert download.last_successful_download == now_ # Download should be retried after the DEFAULT_RETRY_FREQUENCY. expected = datetime(2020, 1, 1, 3, 0, 0, 997200, tzinfo=pytz.UTC) @@ -235,7 +235,7 @@ async def test_recurring_downloads(test_session, test_download_manager, fake_now await test_download_manager.wait_for_all_downloads() test_downloader.do_download.assert_called_once() download = test_session.query(Download).one() - assert download.is_complete, download.status + assert download.is_complete, download.status_code assert download.last_successful_download == now_ # Floats cause slightly wrong date. assert download.next_download == datetime(2020, 1, 1, 5, 0, 0, 997200, tzinfo=pytz.UTC) diff --git a/wrolpi/test/test_root_api.py b/wrolpi/test/test_root_api.py index 2d5737d5..fbd382a3 100644 --- a/wrolpi/test/test_root_api.py +++ b/wrolpi/test/test_root_api.py @@ -4,6 +4,7 @@ from itertools import zip_longest import pytest from mock import mock +from sanic import Request from wrolpi.admin import HotspotStatus from wrolpi.api_utils import json_error_handler @@ -258,8 +259,8 @@ def test_hotspot_settings(test_session, test_client, test_config): request, response = test_client.patch('/api/settings', content=json.dumps(content)) assert response.status_code == HTTPStatus.BAD_REQUEST assert response.json == {'code': 'HOTSPOT_PASSWORD_TOO_SHORT', - 'error': '', - 'summary': 'Hotspot password must be at least 8 characters' + 'error': 'Hotspot password must be at least 8 characters', + 'message': 'Hotspot password must be at least 8 characters' } @@ -439,12 +440,12 @@ async def test_restart_download(test_session, test_async_client, test_download_m request, response = await test_async_client.post(f'/api/download/{download.id}/restart') assert response.status_code == HTTPStatus.NO_CONTENT download = test_session.query(Download).one() - assert download.is_new, download.status + assert download.is_new, download.status_code # Wait for the background download to fail. It should be deferred. await test_download_manager.wait_for_all_downloads() download = test_session.query(Download).one() - assert download.is_deferred, download.status + assert download.is_deferred, download.status_code def test_get_global_statistics(test_session, test_client): @@ -588,6 +589,18 @@ async def test_search_file_estimates(test_session, test_async_client, archive_fa await assert_results(dict(from_year=2000, to_year=2000), 2) +@pytest.mark.asyncio +async def test_external_error(test_async_client): + """A non-SanicException error is handled as an Internal Server Error.""" + @test_async_client.sanic_app.get('/error') + async def error(_: Request): + raise RuntimeError('Oh no!') + + request, response = await test_async_client.get('/error') + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.json == {'code': 'RuntimeError', 'error': 'Oh no!', 'message': None} + + def test_recursive_errors(): """Errors are reported recursively.""" @@ -597,14 +610,14 @@ def test_recursive_errors(): def two(): try: one() - except Exception as e: - raise SearchEmpty('A more specific error') from e + except Exception as e1: + raise SearchEmpty('A more specific error') from e1 def three(): try: two() - except Exception as e: - raise ValidationError('A broad error') from e + except Exception as e2: + raise ValidationError('A broad error') from e2 try: three() @@ -613,15 +626,15 @@ def test_recursive_errors(): assert json.loads(response.body.decode()) == { 'code': 'VALIDATION_ERROR', 'error': 'A broad error', - 'summary': 'Could not validate the contents of the request', + 'message': 'Could not validate the contents of the request', 'cause': { 'code': 'SEARCH_EMPTY', 'error': 'A more specific error', - 'summary': 'Search is empty, search_str must have content.', + 'message': 'Search is empty, search_str must have content.', 'cause': { - 'code': None, + 'code': 'ValueError', # Code falls back to Exception type. 'error': 'Some error outside WROLPi', - 'summary': None, + 'message': None, }, }, }