See P2007 for discussion. This is probably arising from the change in D16489.
Description
Description
Revisions and Commits
Revisions and Commits
Related Objects
Related Objects
Event Timeline
Comment Actions
@chad, I'm having trouble reproducing the patch from P2007 not working. Here's what I did:
- Set mysql.pass to a bad value (other values are correct).
- Ran bin/config.
With master, I get an error similar to yours:
Unpatched
$ ./bin/config [2016-09-05 08:30:41] EXCEPTION: (AphrontInvalidCredentialsQueryException) #1045: Access denied for user 'root'@'localhost' (using password: YES) at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:321] arcanist(head=master, ref.master=9e82ef979e81), corgi(head=master, ref.master=5b9171222bc9), instances(head=stable, ref.master=485bc8128198, ref.stable=2983bc917601), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=master, ref.master=31c5f39506bd), phutil(head=stable, ref.master=97f05269fdb1, ref.stable=c14343ee620e), services(head=stable, ref.master=1fcb5cdb7582, ref.stable=2d8088a5b4b3) #0 AphrontBaseMySQLDatabaseConnection::throwCommonException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:334] #1 AphrontBaseMySQLDatabaseConnection::throwConnectionException(integer, string, string, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php:72] #2 AphrontMySQLiDatabaseConnection::connect() called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:101] #3 AphrontBaseMySQLDatabaseConnection::establishConnection() called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:124] #4 AphrontBaseMySQLDatabaseConnection::requireConnection() called at [<phutil>/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php:15] #5 AphrontMySQLiDatabaseConnection::escapeBinaryString(string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php:11] #6 AphrontMySQLiDatabaseConnection::escapeUTF8String(string) called at [<phutil>/src/xsprintf/qsprintf.php:178] #7 xsprintf_query(AphrontMySQLiDatabaseConnection, string, integer, string, integer) called at [<phutil>/src/xsprintf/xsprintf.php:70] #8 xsprintf(string, AphrontMySQLiDatabaseConnection, array) called at [<phutil>/src/xsprintf/qsprintf.php:64] #9 qsprintf(AphrontMySQLiDatabaseConnection, string, string, string, string) #10 call_user_func_array(string, array) called at [<phutil>/src/xsprintf/queryfx.php:5] #11 queryfx(AphrontMySQLiDatabaseConnection, string, string, string, string) #12 call_user_func_array(string, array) called at [<phutil>/src/xsprintf/queryfx.php:13] #13 queryfx_all(AphrontMySQLiDatabaseConnection, string, string, string, string) #14 call_user_func_array(string, array) called at [<phutil>/src/aphront/storage/connection/AphrontDatabaseConnection.php:36] #15 AphrontDatabaseConnection::queryData(string, string, string, string) #16 call_user_func_array(array, array) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:535] #17 LiskDAO::loadRawDataWhere(string, string) #18 call_user_func_array(array, array) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:476] #19 LiskDAO::loadAllWhere(string, string) called at [<phabricator>/src/infrastructure/env/PhabricatorConfigDatabaseSource.php:19] #20 PhabricatorConfigDatabaseSource::loadConfig(string) called at [<phabricator>/src/infrastructure/env/PhabricatorConfigDatabaseSource.php:7] #21 PhabricatorConfigDatabaseSource::__construct(string) called at [<phabricator>/src/infrastructure/env/PhabricatorEnv.php:232] #22 PhabricatorEnv::buildConfigurationSourceStack() called at [<phabricator>/src/infrastructure/env/PhabricatorEnv.php:95] #23 PhabricatorEnv::initializeCommonEnvironment() called at [<phabricator>/src/infrastructure/env/PhabricatorEnv.php:75] #24 PhabricatorEnv::initializeScriptEnvironment() called at [<phabricator>/scripts/__init_script__.php:21] #25 init_phabricator_script() called at [<phabricator>/scripts/__init_script__.php:24] #26 require_once(string) called at [<phabricator>/scripts/setup/manage_config.php:5]
If I apply this patch:
commit 805b5ae8377ff47b866e81e352ebc748cbf61d03
Author: epriestley <git@epriestley.com>
Date: Mon Sep 5 08:30:30 2016 -0700
WIP
diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php
index d66e022..293383b 100644
--- a/src/infrastructure/env/PhabricatorEnv.php
+++ b/src/infrastructure/env/PhabricatorEnv.php
@@ -235,6 +235,8 @@ final class PhabricatorEnv extends Phobject {
// If the database is not available, just skip this configuration
// source. This happens during `bin/storage upgrade`, `bin/conf` before
// schema setup, etc.
+ } catch (AphrontInvalidCredentialsQueryException $exception) {
+ // Patch test.
}
}...things work:
Patched
$ ./bin/config NAME config - manage configuration SYNOPSIS config command [options] Manage Phabricator configuration. WORKFLOWS delete key Delete a local configuration value. get key Get a local configuration value. help [command] Show this help, or workflow help for command. list List all configuration keys. migrate Migrate file-based configuration to more modern storage. set key value Set a local configuration value. Use help command for a detailed command reference. Use --show-standard-options to show additional options.
Are you seeing the same error with the patch?
I don't immediately see how a AphrontInvalidCredentialsQueryException can possibly escape that stack with the patch applied.
Comment Actions
Ah, okay, that's actually consistent with what I'm seeing -- I think I repro'd the web version. You do sometimes need to restart the web thing to get this flow to work fully (we run some special, more-helpful checks during setup, which is only run after restart), but I believe it doesn't matter in this case since the setup checks are a little buggy.
Comment Actions
This is somewhat involved to untangle because the old logic was actually wrong and had a small chance of executing a page incorrectly.
If only the connection to the Config database failed and every other connection on the page worked, we could possibly have run a page without any database config. And there's a tiny chance that this could violate policies or do other bad things.
I technically "fixed" this by accident and I think it's virtually impossible for it to happen in real life, but I don't want to just put things back the way they were since it could happen and could be bad if it did.
I also don't love showing the database setup screen on intermittent database failures. This isn't a big deal, but feels a little rough if you hit it as a user (or, e.g., in the Phacility cluster), and leaks some not-really-sensitive but icky-looking metadata about the connection (host, port, user, whether a password is set or not). It would be nice to handle this a little more cleanly and show a pretty "Fail Whale" / "Serverbeast" sort of page if we're pretty sure the user isn't an administrator doing first-time setup.
I'll fiddle with this a bit and see if I can get things sequencing more cleanly.
This also impacts T11044 (and, specifically, T10759), so the actual path forward might be something like:
- Fix this crudely (rough NUX for first-time administrators).
- Fix T11044 / T10759 (make setup checks handle cluster configurations and partitioning).
- Refine setup vs in-flight explosion versions of this page to target their audiences a little more carefully.
I'm going to dig through some other stuff in my queue first since this stuff could be somewhat involved. Current NUX is definitely pretty rough for new admins though I'm hoping to get immediate issue fixed today, at least.
Comment Actions
@epriestley : Ouch! - Alternatively since all of you are already on it. What git commit can i revert to quickly fix this? (Note it was a clean clone i am trying to re-setup)
Comment Actions
You can manually edit conf/local/local.json instead of using bin/config set.
There is no clean commit to revert that only fixes this without negative side effects, so I don't recommend reverting.
Comment Actions
@epriestley : Alright then, Thanks.
Off my mind perhaps add a --local parameter option to allow skipping all the various Config DB checks.
Comment Actions
Here's my general plan of attack here:
- Split setup checks into "preflight" and "normal" setup checks.
- "Preflight" checks run before configuration is loaded and will be super-core stuff (like PHP version and extensions).
- Initialization sequence will become:
- preflight checks;
- load configuration;
- normal checks;
- actual page.
- All of the check phases will be able to raise setup issues.
Comment Actions
I believe this is fixed in HEAD of master now. Let me know if things look good on your end and I'll cherry-pick the fixes to stable?
Comment Actions
Someone in IRC hit this on stable and I haven't seen more reports on master, so I'm going to bring the changes over.