diff --git a/docs/errors.md b/docs/errors.md index 7842aaa..398e968 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -57,7 +57,7 @@ Fired on every loaded AMX when a threaded query fails. Parameters: | Parameter | Type | Description | |---|---|---| -| `errorid` | int | MySQL server error code (1062, 1045, 1064, …) or `0` for transport-level errors (TCP drop, IO error) | +| `errorid` | int | MySQL server error code (1062, 1045, 1064, …) or `0` when the statement never reached the server — a dropped connection, an IO error, or a `.sql` file that could not be read | | `error` | string | Full error text from the `mysql` crate | | `callback` | string | Name of the public the query asked for (empty if fire-and-forget) | | `query` | string | Exact SQL that was sent to the server. **Printing it puts the whole statement in your server log** — see the note below | diff --git a/src/callback.rs b/src/callback.rs index 39b4c2f..ff47947 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -87,14 +87,18 @@ pub fn fire_on_query_error( None => continue, }; + // The macro pushes its arguments in reverse (the AMX stack + // convention), so the order listed here is the order the public + // receives them in. It must match the forward: + // OnQueryError(errorid, error[], callback[], query[], connId) let _ = exec_public!( amx, "OnQueryError", - conn_id, - query => string, - callback => string, + error_id, error_msg => string, - error_id + callback => string, + query => string, + conn_id ); } } diff --git a/src/connection.rs b/src/connection.rs index 5a90217..b8d75e5 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -602,7 +602,7 @@ fn collect_result( Err(e) => { return Err(QueryError { code: extract_mysql_errno(&e), - message: e.to_string(), + message: describe_mysql_error(&e), }); } } @@ -649,7 +649,7 @@ pub fn execute_query(conn: &mut PooledConn, query: &str) -> Result u16 { } } +/// The text to hand a gamemode for a failed query. +/// +/// `mysql::Error`'s `Display` wraps the server's own message in the variant +/// name, so a missing table reached `OnQueryError` as +/// `MySqlError { ERROR 1146 (42S02): Table 'db.t' doesn't exist }` - Rust +/// internals in a Pawn string. For a server-side error the message alone is +/// what the script wants; everything else (IO, TLS, protocol) keeps the full +/// text, which is all the detail there is. +fn describe_mysql_error(err: &mysql::Error) -> String { + match err { + mysql::Error::MySqlError(e) => e.message.clone(), + other => other.to_string(), + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/natives/query.rs b/src/natives/query.rs index b846910..c9ebc67 100644 --- a/src/natives/query.rs +++ b/src/natives/query.rs @@ -3,6 +3,7 @@ use samp::cell::Ref; use samp::native; use samp::prelude::*; +use crate::callback; use crate::connection::{EscapeMode, escape_string}; use crate::error::{ErrorState, MysqlError}; use crate::logger::Logger; @@ -182,9 +183,11 @@ impl MysqlPlugin { let script = match std::fs::read_to_string(&path) { Ok(script) => script, Err(err) => { + // The path is resolved against the server's working directory, + // which is the server root - not `scriptfiles/`. Getting that + // wrong is the common mistake here, so say where we looked. let msg = format!("mysql_query_file: could not read '{path}': {err}"); - Logger::warn(&msg); - self.connections.global_error = ErrorState::new(MysqlError::QueryFailed, msg); + self.report_script_failure(&msg, &path, &callback, conn_id); return false; } }; @@ -192,8 +195,7 @@ impl MysqlPlugin { let statements = crate::sql::split_statements(&script); if statements.is_empty() { let msg = format!("mysql_query_file: '{path}' contains no statements."); - Logger::warn(&msg); - self.connections.global_error = ErrorState::new(MysqlError::QueryFailed, msg); + self.report_script_failure(&msg, &path, &callback, conn_id); return false; } @@ -305,6 +307,25 @@ impl MysqlPlugin { } i32::try_from(output.len()).unwrap_or(i32::MAX) } + + /// Reports a `mysql_query_file` failure that happens before any statement + /// is submitted - the file could not be read, or held nothing to run. + /// + /// These used to be a log line and a `false` return, which a script that + /// fires the native and forgets it - the documented, callback-free way to + /// run a schema - never sees. A statement that fails *after* submission + /// reaches `OnQueryError`, so the two halves of the same operation + /// reported through different channels. Now both go to `OnQueryError`, + /// with the path as the "query" so the message names the file. + fn report_script_failure(&mut self, msg: &str, path: &str, callback: &str, conn_id: i32) { + Logger::warn(msg); + self.connections.global_error = ErrorState::new(MysqlError::QueryFailed, msg.to_string()); + // `errorid` is documented as the MySQL server's own code, or 0 when the + // failure never reached the server. Nothing was sent here, so 0 it is - + // inventing a plugin-specific number would break that contract. The + // per-connection errno still reports MYSQL_ERROR_QUERY_FAILED. + callback::fire_on_query_error(&self.amx_list, 0, msg, callback, path, conn_id); + } } /// Parses variadic callback parameters based on the format string.