From 83d0a9bb80f62f2a5d73d04c150647dad7a7b341 Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Sat, 31 Jul 2021 15:24:23 +1000 Subject: [PATCH] Better error messages --- build_wasm.sh | 4 +- html/index.js | 6 +++ html/worker.js | 126 ++++++++++++++++++++++++------------------- src/election.rs | 27 +++++++--- src/main.rs | 82 ++++++++++++++++++++-------- src/parser/blt.rs | 6 +++ src/stv/mod.rs | 32 ++++++----- src/stv/wasm.rs | 39 ++++++++++---- tests/aec.rs | 2 +- tests/constraints.rs | 15 ++---- tests/meek.rs | 8 +-- tests/scotland.rs | 4 +- tests/utils/mod.rs | 8 +-- 13 files changed, 226 insertions(+), 133 deletions(-) diff --git a/build_wasm.sh b/build_wasm.sh index e569d50..02ffaa5 100755 --- a/build_wasm.sh +++ b/build_wasm.sh @@ -4,9 +4,9 @@ PATH=$PATH:$HOME/.cargo/bin # Build cargo PROFILE=${1:-release} if [ $PROFILE == 'debug' ]; then - cargo build --lib --target wasm32-unknown-unknown + cargo build --lib --target wasm32-unknown-unknown || exit 1 else - cargo build --lib --target wasm32-unknown-unknown --$PROFILE + cargo build --lib --target wasm32-unknown-unknown --$PROFILE || exit 1 fi # Apply wasm-bindgen diff --git a/html/index.js b/html/index.js index 97d2b43..f3891fd 100644 --- a/html/index.js +++ b/html/index.js @@ -97,6 +97,9 @@ worker.onmessage = function(evt) { response = window.prompt(evt.data.message); } worker.postMessage({'type': 'userInput', 'response': response}); + + } else if (evt.data.type === 'errorMessage') { + divLogs2.insertAdjacentHTML('beforeend', evt.data.message); } } @@ -155,6 +158,9 @@ async function clickCount() { parseInt(document.getElementById('txtPPDP').value), ]; + // Reset UI + document.getElementById('printPane').style.display = 'none'; + divLogs2.innerHTML = ''; // Might have error messages from previous execution // Dispatch to worker worker.postMessage({ diff --git a/html/worker.js b/html/worker.js index eff16a3..822699a 100644 --- a/html/worker.js +++ b/html/worker.js @@ -20,63 +20,73 @@ initWasm(); var numbers, election, opts, state, stageNum; onmessage = function(evt) { - if (evt.data.type === 'countElection') { - if (evt.data.numbers === 'fixed') { - numbers = 'Fixed'; - wasm.fixed_set_dps(evt.data.decimals); - } else if (evt.data.numbers === 'gfixed') { - numbers = 'GuardedFixed'; - wasm.gfixed_set_dps(evt.data.decimals); - } else if (evt.data.numbers === 'float64') { - numbers = 'NativeFloat64'; - } else if (evt.data.numbers === 'rational') { - numbers = 'Rational'; + try { + if (evt.data.type === 'countElection') { + errored = false; + + if (evt.data.numbers === 'fixed') { + numbers = 'Fixed'; + wasm.fixed_set_dps(evt.data.decimals); + } else if (evt.data.numbers === 'gfixed') { + numbers = 'GuardedFixed'; + wasm.gfixed_set_dps(evt.data.decimals); + } else if (evt.data.numbers === 'float64') { + numbers = 'NativeFloat64'; + } else if (evt.data.numbers === 'rational') { + numbers = 'Rational'; + } else { + throw 'Unknown --numbers'; + } + + // Init election + election = wasm['election_from_blt_' + numbers](evt.data.bltData); + + if (evt.data.normaliseBallots) { + wasm['election_normalise_ballots_' + numbers](election); + } + + // Init constraints if applicable + if (evt.data.conData) { + wasm['election_load_constraints_' + numbers](election, evt.data.conData); + } + + // Init STV options + opts = wasm.STVOptions.new.apply(null, evt.data.optsStr); + + // Validate options + opts.validate(); + + // Describe count + postMessage({'type': 'describeCount', 'content': wasm['describe_count_' + numbers](evt.data.bltPath, election, opts)}); + + // Init results table + postMessage({'type': 'initResultsTable', 'content': wasm['init_results_table_' + numbers](election, opts)}); + + // Step election + state = wasm['CountState' + numbers].new(election); + wasm['count_init_' + numbers](state, opts); + + postMessage({'type': 'updateResultsTable', 'result': wasm['update_results_table_' + numbers](1, state, opts)}); + postMessage({'type': 'updateStageComments', 'comment': wasm['update_stage_comments_' + numbers](state)}); + + stageNum = 2; + + resumeCount(); + + } else if (evt.data.type == 'userInput') { + userInputBuffer = evt.data.response; + + // Rewind the stack + // Asyncify will retrace the function calls in the stack until again reaching get_user_input + wasmRaw.asyncify_start_rewind(DATA_ADDR); + resumeCount(); + } + } catch (ex) { + if (errored) { + // Panic already logged and sent to UI } else { - throw 'Unknown --numbers'; + throw ex; } - - // Init election - election = wasm['election_from_blt_' + numbers](evt.data.bltData); - - if (evt.data.normaliseBallots) { - wasm['election_normalise_ballots_' + numbers](election); - } - - // Init constraints if applicable - if (evt.data.conData) { - wasm['election_load_constraints_' + numbers](election, evt.data.conData); - } - - // Init STV options - opts = wasm.STVOptions.new.apply(null, evt.data.optsStr); - - // Validate options - opts.validate(); - - // Describe count - postMessage({'type': 'describeCount', 'content': wasm['describe_count_' + numbers](evt.data.bltPath, election, opts)}); - - // Init results table - postMessage({'type': 'initResultsTable', 'content': wasm['init_results_table_' + numbers](election, opts)}); - - // Step election - state = wasm['CountState' + numbers].new(election); - wasm['count_init_' + numbers](state, opts); - - postMessage({'type': 'updateResultsTable', 'result': wasm['update_results_table_' + numbers](1, state, opts)}); - postMessage({'type': 'updateStageComments', 'comment': wasm['update_stage_comments_' + numbers](state)}); - - stageNum = 2; - - resumeCount(); - - } else if (evt.data.type == 'userInput') { - userInputBuffer = evt.data.response; - - // Rewind the stack - // Asyncify will retrace the function calls in the stack until again reaching get_user_input - wasmRaw.asyncify_start_rewind(DATA_ADDR); - resumeCount(); } } @@ -102,6 +112,12 @@ function resumeCount() { postMessage({'type': 'finalResultSummary', 'summary': wasm['final_result_summary_' + numbers](state, opts)}); } +var errored = false; +function wasm_error(message) { + postMessage({'type': 'errorMessage', 'message': message}); + errored = true; +} + var userInputBuffer = null; function get_user_input(message) { diff --git a/src/election.rs b/src/election.rs index 0182797..86ed6af 100644 --- a/src/election.rs +++ b/src/election.rs @@ -18,12 +18,21 @@ use crate::constraints::{Constraints, ConstraintMatrix}; use crate::logger::Logger; use crate::numbers::Number; -use crate::parser::blt::BLTParser; +use crate::parser::blt::{BLTParser, ParseError}; use crate::sharandom::SHARandom; use std::collections::HashMap; use std::iter::Peekable; +#[cfg(not(target_arch = "wasm32"))] +use utf8_chars::BufReadCharsExt; +#[cfg(not(target_arch = "wasm32"))] +use std::fs::File; +#[cfg(not(target_arch = "wasm32"))] +use std::io::BufReader; +#[cfg(not(target_arch = "wasm32"))] +use std::path::Path; + /// An election to be counted pub struct Election { /// Name of the election @@ -42,12 +51,18 @@ pub struct Election { impl Election { /// Parse the given BLT file and return an [Election] - pub fn from_blt>(input: Peekable) -> Self { + pub fn from_blt>(input: Peekable) -> Result { let mut parser = BLTParser::new(input); - match parser.parse_blt() { - Ok(_) => { return parser.as_election(); } - Err(e) => { panic!("Syntax Error: {}", e); } - } + parser.parse_blt()?; + return Ok(parser.as_election()); + } + + /// Parse the BLT file at the given path and return an [Election] + #[cfg(not(target_arch = "wasm32"))] + pub fn from_file>(path: P) -> Result { + let mut reader = BufReader::new(File::open(path).expect("IO Error")); + let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); + return Ok(Election::from_blt(chars)?); } /// Convert ballots with weight >1 to multiple ballots of weight 1 diff --git a/src/main.rs b/src/main.rs index ed0bdd6..843243f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,11 +21,10 @@ use opentally::numbers::{Fixed, GuardedFixed, NativeFloat64, Number, Rational}; use opentally::stv; use clap::{AppSettings, Clap}; -use utf8_chars::BufReadCharsExt; use std::cmp::max; use std::fs::File; -use std::io::{self, BufRead, BufReader}; +use std::io::{self, BufRead}; use std::ops; /// Open-source election vote counting @@ -186,36 +185,54 @@ struct STV { } fn main() { + match main_() { + Ok(_) => {} + Err(code) => { + std::process::exit(code); + } + } +} + +fn main_() -> Result<(), i32> { // Read arguments let opts: Opts = Opts::parse(); let Command::STV(cmd_opts) = opts.command; - // Read BLT file - let mut reader = BufReader::new(File::open(&cmd_opts.filename).expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - - // Create and count election according to --numbers + // Read and count election according to --numbers if cmd_opts.numbers == "rational" { - let mut election: Election = Election::from_blt(chars); + let mut election = election_from_file(&cmd_opts.filename)?; maybe_load_constraints(&mut election, &cmd_opts.constraints); // Must specify :: here and in a few other places because ndarray causes E0275 otherwise - count_election::(election, cmd_opts); + count_election::(election, cmd_opts)?; } else if cmd_opts.numbers == "float64" { - let mut election: Election = Election::from_blt(chars); + let mut election = election_from_file(&cmd_opts.filename)?; maybe_load_constraints(&mut election, &cmd_opts.constraints); - count_election::(election, cmd_opts); + count_election::(election, cmd_opts)?; } else if cmd_opts.numbers == "fixed" { Fixed::set_dps(cmd_opts.decimals); - let mut election: Election = Election::from_blt(chars); + + let mut election = election_from_file(&cmd_opts.filename)?; maybe_load_constraints(&mut election, &cmd_opts.constraints); - count_election::(election, cmd_opts); + count_election::(election, cmd_opts)?; } else if cmd_opts.numbers == "gfixed" { GuardedFixed::set_dps(cmd_opts.decimals); - let mut election: Election = Election::from_blt(chars); + let mut election = election_from_file(&cmd_opts.filename)?; maybe_load_constraints(&mut election, &cmd_opts.constraints); - count_election::(election, cmd_opts); + count_election::(election, cmd_opts)?; + } + + return Ok(()); +} + +fn election_from_file(path: &str) -> Result, i32> { + match Election::from_file(path) { + Ok(e) => return Ok(e), + Err(err) => { + println!("Syntax Error: {}", err); + return Err(1); + } } } @@ -227,7 +244,7 @@ fn maybe_load_constraints(election: &mut Election, constraints: &O } } -fn count_election(mut election: Election, cmd_opts: STV) +fn count_election(mut election: Election, cmd_opts: STV) -> Result<(), i32> where for<'r> &'r N: ops::Sub<&'r N, Output=N>, for<'r> &'r N: ops::Mul<&'r N, Output=N>, @@ -263,7 +280,13 @@ where ); // Validate options - stv_opts.validate(); + match stv_opts.validate() { + Ok(_) => {} + Err(err) => { + println!("Error: {}", err.describe()); + return Err(1); + } + } // Describe count let total_ballots = election.ballots.iter().fold(N::zero(), |acc, b| { acc + &b.orig_value }); @@ -285,15 +308,30 @@ where let mut state = CountState::new(&election); // Distribute first preferences - stv::count_init(&mut state, &stv_opts).unwrap(); + match stv::count_init(&mut state, &stv_opts) { + Ok(_) => {} + Err(err) => { + println!("Error: {}", err.describe()); + return Err(1); + } + } + let mut stage_num = 1; print_stage(stage_num, &state, &cmd_opts); loop { - let is_done = stv::count_one_stage(&mut state, &stv_opts); - if is_done.unwrap() { - break; + match stv::count_one_stage(&mut state, &stv_opts) { + Ok(is_done) => { + if is_done { + break; + } + } + Err(err) => { + println!("Error: {}", err.describe()); + return Err(1); + } } + stage_num += 1; print_stage(stage_num, &state, &cmd_opts); } @@ -315,6 +353,8 @@ where println!("{}. {}", i + 1, winner.name); } } + + return Ok(()); } fn print_candidates<'a, N: 'a + Number, I: Iterator)>>(candidates: I, cmd_opts: &STV) { diff --git a/src/parser/blt.rs b/src/parser/blt.rs index d64a36a..69ddb4f 100644 --- a/src/parser/blt.rs +++ b/src/parser/blt.rs @@ -61,6 +61,12 @@ impl fmt::Display for ParseError { } } +impl fmt::Debug for ParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + return fmt::Display::fmt(self, f); + } +} + impl> BLTParser { // NON-TERMINALS - HIGHER LEVEL diff --git a/src/stv/mod.rs b/src/stv/mod.rs index 8884c6d..8f60850 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -36,6 +36,7 @@ use itertools::Itertools; use wasm_bindgen::prelude::wasm_bindgen; use std::collections::HashMap; +use std::fmt; use std::ops; /// Options for conducting an STV count @@ -228,11 +229,12 @@ impl STVOptions { } /// Validate the combination of [STVOptions] and panic if invalid - pub fn validate(&self) { + pub fn validate(&self) -> Result<(), STVError> { if self.surplus == SurplusMethod::Meek { - if self.transferable_only { panic!("--surplus meek is incompatible with --transferable-only"); } - if self.exclusion != ExclusionMethod::SingleStage { panic!("--surplus meek requires --exclusion single_stage"); } + if self.transferable_only { return Err(STVError::InvalidOptions("--surplus meek is incompatible with --transferable-only")); } + if self.exclusion != ExclusionMethod::SingleStage { return Err(STVError::InvalidOptions("--surplus meek requires --exclusion single_stage")); } } + return Ok(()); } } @@ -431,25 +433,31 @@ impl ConstraintMode { } /// An error during the STV count -#[wasm_bindgen] #[derive(Debug)] pub enum STVError { - /// User input is required - RequireInput, + /// Options for the count are invalid + InvalidOptions(&'static str), /// Tie could not be resolved UnresolvedTie, } impl STVError { - /// Return the name of the error as a string - pub fn name(&self) -> &'static str { + /// Describe the error + pub fn describe(&self) -> &'static str { match self { - STVError::RequireInput => "RequireInput", - STVError::UnresolvedTie => "UnresolvedTie", + STVError::InvalidOptions(s) => s, + STVError::UnresolvedTie => "Unable to resolve tie", } } } +impl fmt::Display for STVError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.describe())?; + return Ok(()); + } +} + /// Distribute first preferences, and initialise other states such as the random number generator and tie-breaking rules pub fn count_init<'a, N: Number>(state: &mut CountState<'a, N>, opts: &'a STVOptions) -> Result where @@ -1288,7 +1296,7 @@ fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, c } } } - panic!("Unable to resolve tie"); + return Err(STVError::UnresolvedTie); } /// Break a tie between the given candidates according to [STVOptions::ties], selecting the lowest candidate @@ -1309,7 +1317,7 @@ fn choose_lowest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, ca } } } - panic!("Unable to resolve tie"); + return Err(STVError::UnresolvedTie); } /// If required, initialise the state of the forwards or backwards tie-breaking strategies, according to [STVOptions::ties] diff --git a/src/stv/wasm.rs b/src/stv/wasm.rs index a7c9473..e7de324 100644 --- a/src/stv/wasm.rs +++ b/src/stv/wasm.rs @@ -16,6 +16,7 @@ */ #![allow(rustdoc::private_intra_doc_links)] +#![allow(unused_unsafe)] // Confuses cargo check use crate::constraints::Constraints; use crate::election::{CandidateState, CountState, Election}; @@ -25,10 +26,24 @@ use crate::stv; extern crate console_error_panic_hook; use js_sys::Array; -use wasm_bindgen::{JsValue, prelude::wasm_bindgen}; +use wasm_bindgen::prelude::wasm_bindgen; use std::cmp::max; +// Error handling + +#[wasm_bindgen] +extern "C" { + fn wasm_error(message: String); +} + +macro_rules! wasm_error { + ($type:expr, $err:expr) => { { + unsafe { wasm_error(format!("{}: {}", $type, $err)); } + panic!("{}: {}", $type, $err); + } } +} + // Init /// Wrapper for [Fixed::set_dps] @@ -56,7 +71,10 @@ macro_rules! impl_type { // Install panic! hook console_error_panic_hook::set_once(); - let election: Election<$type> = Election::from_blt(text.chars().peekable()); + let election: Election<$type> = match Election::from_blt(text.chars().peekable()) { + Ok(e) => e, + Err(err) => wasm_error!("Syntax Error", err), + }; return [](election); } @@ -77,20 +95,20 @@ macro_rules! impl_type { /// Wrapper for [stv::count_init] #[wasm_bindgen] #[allow(non_snake_case)] - pub fn [](state: &mut [], opts: &STVOptions) -> Result { + pub fn [](state: &mut [], opts: &STVOptions) -> bool { match stv::count_init(&mut state.0, opts.as_static()) { - Ok(v) => Ok(v), - Err(e) => Err(e.name().into()), + Ok(v) => v, + Err(err) => wasm_error!("Error", err), } } /// Wrapper for [stv::count_one_stage] #[wasm_bindgen] #[allow(non_snake_case)] - pub fn [](state: &mut [], opts: &STVOptions) -> Result { + pub fn [](state: &mut [], opts: &STVOptions) -> bool { match stv::count_one_stage::<[<$type>]>(&mut state.0, &opts.0) { - Ok(v) => Ok(v), - Err(e) => Err(e.name().into()), + Ok(v) => v, + Err(err) => wasm_error!("Error", err), } } @@ -248,7 +266,10 @@ impl STVOptions { /// Wrapper for [stv::STVOptions::validate] pub fn validate(&self) { - self.0.validate(); + match self.0.validate() { + Ok(_) => {} + Err(err) => { wasm_error!("Error", err) } + } } } diff --git a/tests/aec.rs b/tests/aec.rs index 185956e..28b15a9 100644 --- a/tests/aec.rs +++ b/tests/aec.rs @@ -52,7 +52,7 @@ fn aec_tas19_rational() { let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); // Read BLT - let election: Election = Election::from_blt(chars); + let election: Election = Election::from_blt(chars).expect("Syntax Error"); // Validate candidate names for (i, candidate) in candidates.iter().enumerate() { diff --git a/tests/constraints.rs b/tests/constraints.rs index d74cec6..7d5d649 100644 --- a/tests/constraints.rs +++ b/tests/constraints.rs @@ -21,19 +21,16 @@ use opentally::constraints::Constraints; use opentally::election::{CandidateState, CountState, Election}; use opentally::numbers::Rational; use opentally::stv; -use utf8_chars::BufReadCharsExt; use std::fs::File; -use std::io::{self, BufRead, BufReader}; +use std::io::{self, BufRead}; #[test] fn prsa1_constr1_rational() { // FIXME: This is unvalidated! // Read BLT - let mut reader = BufReader::new(File::open("tests/data/prsa1.blt").expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - let mut election: Election = Election::from_blt(chars); + let mut election: Election = Election::from_file("tests/data/prsa1.blt").expect("Syntax Error"); // Read CON let file = File::open("tests/data/prsa1_constr1.con").expect("IO Error"); @@ -94,9 +91,7 @@ fn prsa1_constr2_rational() { // FIXME: This is unvalidated! // Read BLT - let mut reader = BufReader::new(File::open("tests/data/prsa1.blt").expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - let mut election: Election = Election::from_blt(chars); + let mut election: Election = Election::from_file("tests/data/prsa1.blt").expect("Syntax Error"); // Read CON let file = File::open("tests/data/prsa1_constr2.con").expect("IO Error"); @@ -157,9 +152,7 @@ fn prsa1_constr3_rational() { // FIXME: This is unvalidated! // Read BLT - let mut reader = BufReader::new(File::open("tests/data/prsa1.blt").expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - let mut election: Election = Election::from_blt(chars); + let mut election: Election = Election::from_file("tests/data/prsa1.blt").expect("Syntax Error"); // Read CON let file = File::open("tests/data/prsa1_constr3.con").expect("IO Error"); diff --git a/tests/meek.rs b/tests/meek.rs index 423cea2..0971015 100644 --- a/tests/meek.rs +++ b/tests/meek.rs @@ -87,9 +87,7 @@ fn meek06_ers97_fixed12() { Fixed::set_dps(12); // Read BLT - let mut reader = BufReader::new(File::open("tests/data/ers97.blt").expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - let election: Election = Election::from_blt(chars); + let election: Election = Election::from_file("tests/data/ers97.blt").expect("Syntax Error"); // Initialise count state let mut state = CountState::new(&election); @@ -161,9 +159,7 @@ fn meeknz_ers97_fixed12() { Fixed::set_dps(12); // Read BLT - let mut reader = BufReader::new(File::open("tests/data/ers97.blt").expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - let election: Election = Election::from_blt(chars); + let election: Election = Election::from_file("tests/data/ers97.blt").expect("Syntax Error"); // Initialise count state let mut state = CountState::new(&election); diff --git a/tests/scotland.rs b/tests/scotland.rs index 0a4a61b..54388b1 100644 --- a/tests/scotland.rs +++ b/tests/scotland.rs @@ -113,9 +113,7 @@ where let num_stages = root.get_child("headerrow").expect("Syntax Error").children.len(); // Read BLT - let mut reader = BufReader::new(File::open("tests/data/linn07.blt").expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - let mut election: Election = Election::from_blt(chars); + let mut election: Election = Election::from_file("tests/data/linn07.blt").expect("Syntax Error"); // !!! FOR SCOTTISH STV !!! election.normalise_ballots(); diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index 61fd980..d4bd874 100644 --- a/tests/utils/mod.rs +++ b/tests/utils/mod.rs @@ -20,10 +20,7 @@ use opentally::numbers::Number; use opentally::stv; use csv::StringRecord; -use utf8_chars::BufReadCharsExt; -use std::fs::File; -use std::io::BufReader; use std::ops; #[allow(dead_code)] // Suppress false positive @@ -49,10 +46,7 @@ where let stages: Vec = records.first().unwrap().iter().skip(1).step_by(2).map(|s| s.parse().unwrap()).collect(); // Read BLT - let mut reader = BufReader::new(File::open(blt_file).expect("IO Error")); - let chars = reader.chars().map(|r| r.expect("IO Error")).peekable(); - - let election: Election = Election::from_blt(chars); + let election: Election = Election::from_file(blt_file).expect("Syntax Error"); // Validate candidate names for (i, candidate) in candidates.iter().enumerate() {