From ed0ca6f331da58916012f8558e41496bcfaeb275 Mon Sep 17 00:00:00 2001 From: Taka Date: Tue, 7 May 2019 07:19:04 +0000 Subject: [PATCH] Tightened up handling of configuration errors. --- vasl_templates/webapp/__init__.py | 4 ++-- vasl_templates/webapp/files.py | 3 +++ vasl_templates/webapp/main.py | 5 +++-- vasl_templates/webapp/roar.py | 14 +++++++------- vasl_templates/webapp/snippets.py | 3 +-- vasl_templates/webapp/static/vassal.js | 2 +- vasl_templates/webapp/tests/remote.py | 2 +- vasl_templates/webapp/utils.py | 5 ++++- vasl_templates/webapp/vasl_mod.py | 26 +++++++++++++++++++------- vasl_templates/webapp/vassal.py | 6 +++--- vasl_templates/webapp/vo_notes.py | 7 +++++-- 11 files changed, 49 insertions(+), 28 deletions(-) diff --git a/vasl_templates/webapp/__init__.py b/vasl_templates/webapp/__init__.py index 517d93e..9f76c32 100644 --- a/vasl_templates/webapp/__init__.py +++ b/vasl_templates/webapp/__init__.py @@ -34,11 +34,11 @@ def _on_startup(): # load the vehicle/ordnance notes from vasl_templates.webapp.vo_notes import load_vo_notes #pylint: disable=cyclic-import - load_vo_notes() + from vasl_templates.webapp.main import startup_msg_store #pylint: disable=cyclic-import + load_vo_notes( startup_msg_store ) # initialize ROAR integration from vasl_templates.webapp.roar import init_roar #pylint: disable=cyclic-import - from vasl_templates.webapp.main import startup_msg_store #pylint: disable=cyclic-import init_roar( startup_msg_store ) # --------------------------------------------------------------------- diff --git a/vasl_templates/webapp/files.py b/vasl_templates/webapp/files.py index d88714e..bda0280 100644 --- a/vasl_templates/webapp/files.py +++ b/vasl_templates/webapp/files.py @@ -5,6 +5,7 @@ import io import urllib.request import urllib.parse import mimetypes +import logging from flask import send_file, send_from_directory, jsonify, redirect, url_for, abort, render_template @@ -62,6 +63,8 @@ def get_user_file( path ): dname = app.config.get( "USER_FILES_DIR" ) if not dname: abort( 404 ) + if not os.path.isdir( dname ): + logging.error( "Missing user files directory: %s", dname ) resp = FileServer( dname ).serve_file( path ) if not resp: abort( 404 ) diff --git a/vasl_templates/webapp/main.py b/vasl_templates/webapp/main.py index b5acf64..851bd30 100644 --- a/vasl_templates/webapp/main.py +++ b/vasl_templates/webapp/main.py @@ -34,8 +34,9 @@ def get_startup_msgs(): try: VassalShim.check_vassal_version( startup_msg_store ) except Exception as ex: #pylint: disable=broad-except - # NOTE: We can get here is VASSAL has been configured, but not Java - don't show an error to the user :-/ - logging.warning( "Can't get the VASSAL version: %s", ex ) + msg = "Can't get the VASSAL version: {}".format( ex ) + logging.error( "%s", msg ) + startup_msg_store.error( msg ) # collect all the startup messages startup_msgs = {} diff --git a/vasl_templates/webapp/roar.py b/vasl_templates/webapp/roar.py index e255c4a..5bbc59b 100644 --- a/vasl_templates/webapp/roar.py +++ b/vasl_templates/webapp/roar.py @@ -53,7 +53,7 @@ def init_roar( msg_store ): # check if we should download the ROAR scenario index if download: if app.config.get("DISABLE_ROAR_SCENARIO_INDEX_DOWNLOAD"): - _logger.warning( "Downloading the ROAR scenario index has been disabled." ) + _logger.critical( "Downloading the ROAR scenario index has been disabled." ) else: # yup - make it so (nb: we do it in a background thread to avoid blocking the startup process) # NOTE: This is the only place we do this, so if it fails, the program needs to be restarted to try again. @@ -73,10 +73,10 @@ def _download_roar_scenario_index( save_fname, msg_store ): data = fp.read().decode( "utf-8" ) except Exception as ex: #pylint: disable=broad-except # NOTE: We catch all exceptions, since we don't want an error here to stop us from running :-/ - error_msg = "Can't download ROAR scenario index: {}".format( getattr(ex,"reason",str(ex)) ) - _logger.warning( error_msg ) + msg = "Can't download the ROAR scenario index: {}".format( getattr(ex,"reason",str(ex)) ) + _logger.warning( "%s", msg ) if msg_store: - msg_store.warning( error_msg ) + msg_store.warning( msg ) return if not _load_roar_scenario_index( data, "downloaded", msg_store ): # NOTE: If we fail to load the scenario index (e.g. because of invalid JSON), we exit here @@ -97,10 +97,10 @@ def _load_roar_scenario_index( data, data_type, msg_store ): scenario_index = json.loads( data ) except Exception as ex: #pylint: disable=broad-except # NOTE: We catch all exceptions, since we don't want an error here to stop us from running :-/ - error_msg = "Can't load {} ROAR scenario index: {}".format( data_type, ex ) - _logger.warning( error_msg ) + msg = "Can't load the {} ROAR scenario index: {}".format( data_type, ex ) + _logger.error( "%s", msg ) if msg_store: - msg_store.warning( error_msg ) + msg_store.error( msg ) return False _logger.debug( "Loaded %s ROAR scenario index OK: #scenarios=%d", data_type, len(scenario_index) ) _logger.debug( "- Last updated: %s", scenario_index.get( "_lastUpdated_", "n/a" ) ) diff --git a/vasl_templates/webapp/snippets.py b/vasl_templates/webapp/snippets.py index ae94484..e4be578 100644 --- a/vasl_templates/webapp/snippets.py +++ b/vasl_templates/webapp/snippets.py @@ -13,7 +13,6 @@ from flask import request, jsonify, send_file, abort from PIL import Image from vasl_templates.webapp import app, globvars -from vasl_templates.webapp.utils import SimpleError from vasl_templates.webapp.config.constants import DATA_DIR from vasl_templates.webapp.webdriver import WebDriver @@ -150,7 +149,7 @@ def make_snippet_image(): try: with WebDriver.get_instance() as webdriver: img = webdriver.get_snippet_screenshot( None, snippet ) - except SimpleError as ex: + except Exception as ex: #pylint: disable=broad-except return "ERROR: {}".format( ex ) # get the image data diff --git a/vasl_templates/webapp/static/vassal.js b/vasl_templates/webapp/static/vassal.js index 1a967f7..64145c1 100644 --- a/vasl_templates/webapp/static/vassal.js +++ b/vasl_templates/webapp/static/vassal.js @@ -76,7 +76,7 @@ function do_update_vsav( vsav_data, fname ) } $("#vassal-shim-error").dialog( { dialogClass: "vassal-shim-error", - title: "Scenario update error", + title: "Can't update the scenario", modal: true, width: 600, height: "auto", open: function() { diff --git a/vasl_templates/webapp/tests/remote.py b/vasl_templates/webapp/tests/remote.py index 0354233..5d07e60 100644 --- a/vasl_templates/webapp/tests/remote.py +++ b/vasl_templates/webapp/tests/remote.py @@ -257,7 +257,7 @@ class ControlTests: _logger.info( "Setting vehicle/ordnance notes: %s", dname ) app.config["CHAPTER_H_NOTES_DIR"] = dname from vasl_templates.webapp.vo_notes import load_vo_notes - load_vo_notes() + load_vo_notes( None ) return self def _set_user_files_dir( self, dtype=None ): diff --git a/vasl_templates/webapp/utils.py b/vasl_templates/webapp/utils.py index acb6172..e05401e 100644 --- a/vasl_templates/webapp/utils.py +++ b/vasl_templates/webapp/utils.py @@ -41,7 +41,10 @@ class MsgStore: def _add_msg( self, msg_type, msg, *args, **kwargs ): """Add a message to the store.""" logger = kwargs.pop( "logger", None ) - msg = msg.format( *args, **kwargs ) + if args or kwargs: + # NOTE: We only format the message if there are any parameters, to handle the case + # where the caller passed us a single string that happens to contain a {. + msg = msg.format( *args, **kwargs ) self._msgs[ msg_type ].append( msg ) if logger: func = getattr( logger, "warn" if msg_type == "warning" else msg_type ) diff --git a/vasl_templates/webapp/vasl_mod.py b/vasl_templates/webapp/vasl_mod.py index 0ec12dc..3c7431b 100644 --- a/vasl_templates/webapp/vasl_mod.py +++ b/vasl_templates/webapp/vasl_mod.py @@ -22,11 +22,20 @@ warnings = [] # nb: for the test suite def set_vasl_mod( vmod_fname, msg_store ): """Install a new global VaslMod object.""" + globvars.vasl_mod = None if vmod_fname: # load and install the specified VASL module extns_dir = app.config.get( "VASL_EXTNS_DIR" ) - extns = _load_vasl_extns( extns_dir ) - globvars.vasl_mod = VaslMod( vmod_fname, DATA_DIR, extns ) + extns = _load_vasl_extns( extns_dir, msg_store ) + try: + vasl_mod = VaslMod( vmod_fname, DATA_DIR, extns ) + except Exception as ex: #pylint: disable=broad-except + msg = "Can't load the VASL module: {}".format( ex ) + _logger.error( "%s", msg ) + if msg_store: + msg_store.error( msg ) + return + globvars.vasl_mod = vasl_mod # make sure the VASL version is one we support if globvars.vasl_mod.vasl_version not in SUPPORTED_VASL_MOD_VERSIONS: if msg_store: @@ -35,20 +44,23 @@ def set_vasl_mod( vmod_fname, msg_store ): globvars.vasl_mod.vasl_version ) ) - else: - # no VASL module has been specified - globvars.vasl_mod = None -def _load_vasl_extns( extn_dir ): #pylint: disable=too-many-locals,too-many-statements,too-many-branches +def _load_vasl_extns( extn_dir, msg_store ): #pylint: disable=too-many-locals,too-many-statements,too-many-branches """Locate VASL extensions and their corresponding vehicle/ordnance info files.""" if not extn_dir: return [] + if not os.path.isdir( extn_dir ): + msg = "Can't find the VASL extensions directory: {}".format( extn_dir ) + _logger.error( "%s", msg ) + if msg_store: + msg_store.error( msg ) + return [] def log_warning( fmt, *args, **kwargs ): #pylint: disable=missing-docstring msg = fmt.format( *args, **kwargs ) warnings.append( msg ) - _logger.warning( msg ) + _logger.warning( "%s", msg ) # load our extension info files all_extn_info = {} diff --git a/vasl_templates/webapp/vassal.py b/vasl_templates/webapp/vassal.py index 15b2829..d8076e0 100644 --- a/vasl_templates/webapp/vassal.py +++ b/vasl_templates/webapp/vassal.py @@ -75,7 +75,7 @@ def update_vsav(): #pylint: disable=too-many-statements vsav_data = fp.read() fname = app.config.get( "UPDATE_VSAV_RESULT" ) # nb: for diagnosing problems if fname: - _logger.debug( "Saving a copy of the update VSAV: %s", fname ) + _logger.debug( "Saving a copy of the updated VSAV: %s", fname ) with open( app.config.get("UPDATE_VSAV_RESULT"), "wb" ) as fp: fp.write( vsav_data ) # read the report @@ -354,8 +354,8 @@ class VassalShim: if version not in SUPPORTED_VASSAL_VERSIONS: if msg_store: msg_store.warning( - "This program has not been tested with VASSAL {}." \ - "

Things might work, but they might not...".format( version ) + "This program has not been tested with VASSAL {}.

Things might work, but they might not...", + version ) # - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/vasl_templates/webapp/vo_notes.py b/vasl_templates/webapp/vo_notes.py index 8444326..c79c984 100644 --- a/vasl_templates/webapp/vo_notes.py +++ b/vasl_templates/webapp/vo_notes.py @@ -28,7 +28,7 @@ def get_ordnance_notes(): # - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -def load_vo_notes(): #pylint: disable=too-many-statements,too-many-locals,too-many-branches +def load_vo_notes( msg_store ): #pylint: disable=too-many-statements,too-many-locals,too-many-branches """Load the Chapter H vehicle/ordnance notes.""" # locate the data directory @@ -39,7 +39,10 @@ def load_vo_notes(): #pylint: disable=too-many-statements,too-many-locals,too-ma # but the user may not have mapped it to a directory outside the container). dname = os.path.abspath( dname ) if not os.path.isdir( dname ): - logging.warning( "Missing Chapter H directory: %s", dname ) + error_msg = "Missing Chapter H directory: {}".format( dname ) + logging.error( "%s", error_msg ) + if msg_store: + msg_store.error( error_msg ) dname = None if not dname: globvars.vo_notes = { "vehicles": {}, "ordnance": {} }