Improve consistency of exception message formatting. #3536

jwage · 2019-04-30T02:24:01Z

Q	A
Type	improvement
BC Break	no
Fixed issues	fixes #3531

Summary

Improve consistency of exception message formatting. See #3531 for details.

TODO

Add tests for exceptions that weren't tested.

lib/Doctrine/DBAL/DBALException.php

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php

lib/Doctrine/DBAL/Exception/MissingSQLParam.php

lib/Doctrine/DBAL/Id/TableGenerator.php

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php

lib/Doctrine/DBAL/Query/Exception/NonUniqueAlias.php

morozov · 2019-04-30T16:46:50Z

@jwage please see the comments above. I haven't reviewed the entire patch yet.

lib/Doctrine/DBAL/DBALException.php

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

lib/Doctrine/DBAL/DBALException.php

lib/Doctrine/DBAL/Driver/PDOStatement.php

tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php

lib/Doctrine/DBAL/Exception/FormatArray.php

lib/Doctrine/DBAL/Schema/Table.php

lib/Doctrine/DBAL/Schema/UniqueConstraint.php

tests/Doctrine/Tests/DBAL/ConnectionTest.php

tests/Doctrine/Tests/DBAL/Exception/InvalidColumnIndexTest.php

tests/Doctrine/Tests/DBAL/Functional/PDOStatementTest.php

lib/Doctrine/DBAL/Exception/MissingSQLType.php

lib/Doctrine/DBAL/Exception/MissingArrayParameterType.php

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

lib/Doctrine/DBAL/Driver/PDOStatement.php

lib/Doctrine/DBAL/Types/Exception/ValueNotConvertible.php

lib/Doctrine/DBAL/Types/Exception/SerializationFailed.php

lib/Doctrine/DBAL/Exception/FormatVariable.php

lib/Doctrine/DBAL/DBALException.php

jwage · 2019-05-01T19:40:20Z

lib/Doctrine/DBAL/Cache/ArrayStatement.php

@@ -48,7 +49,7 @@ public function __construct(array $data)
     */
    public function closeCursor() : void
    {
-        unset($this->data);
+        $this->data = null;


I was adding tests for ArrayStatement and this seemed wrong since it can result in PHP errors if you call other public methods after closeCursor()

Makes perfect sense.

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php

jwage · 2019-05-01T19:51:26Z

lib/Doctrine/DBAL/Sharding/PoolingShardConnection.php

            }

            if (! is_numeric($shard['id']) || $shard['id'] < 1) {
                throw new InvalidArgumentException('Shard Id has to be a non-negative number.');
            }

            if (isset($this->connectionParameters[$shard['id']])) {
-                throw new InvalidArgumentException('Shard ' . $shard['id'] . ' is duplicated in the configuration.');
+                throw new InvalidArgumentException(sprintf('Shard "%s" is duplicated in the configuration.', $shard['id']));


Is $shard['id'] a string?

It's annotated as int|string in most places but looks like it's a numeric string (or an integer):

dbal/lib/Doctrine/DBAL/Sharding/PoolingShardConnection.php

Lines 94 to 96 in 82f5fe3

if (! is_numeric($shard['id']) || $shard['id'] < 1) {

throw new InvalidArgumentException('Shard Id has to be a non-negative number.');

}

Also, the id comes from sys.federation_member_distributions.member_id:

dbal/lib/Doctrine/DBAL/Sharding/SQLAzure/SQLAzureShardManager.php

Lines 158 to 164 in 68e48e8

$sql = 'SELECT member_id as id,

distribution_name as distribution_key,

CAST(range_low AS CHAR) AS rangeLow,

CAST(range_high AS CHAR) AS rangeHigh

FROM sys.federation_member_distributions d

INNER JOIN sys.federations f ON f.federation_id = d.federation_id

WHERE f.name = ' . $this->conn->quote($this->federationName);

which according to this article is INT.

These details are mostly for myself, I'll need this info later to finish the work on strict types.

morozov · 2019-05-06T19:54:18Z

@jwage I rebased develop yesterday to accommodate recent conflicting changes from master. Please rebase.

jwage · 2019-05-07T01:11:48Z

@morozov Done!

morozov · 2019-05-07T18:30:06Z

lib/Doctrine/DBAL/Exception/InvalidPlatformType.php

@@ -30,9 +29,9 @@ public static function new($invalidPlatform) : self

        return new self(
            sprintf(
-                "Option 'platform' must be an object and subtype of '%s'. Got '%s'",
+                'Option "platform" must be an object and subtype of %s. Got "%s".',


No quotes around type needed.

morozov · 2019-05-07T18:30:24Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

@@ -828,7 +829,10 @@ private function convertSingleBooleanValue($value, $callback)
            return $callback(true);
        }

-        throw new UnexpectedValueException("Unrecognized boolean literal '${value}'");
+        throw new UnexpectedValueException(sprintf(
+            'Unrecognized boolean literal, "%s" given.',


No quotes around type needed. Or… most likely it will be a printable scalar value. Let's leave it without the GetVariableType.

morozov · 2019-05-07T18:35:15Z

@jwage thanks. Could you look at the build failure and the new comments? Looks like we're really close to finishing it.

jwage · 2019-05-07T21:30:34Z

Fixed latest feedback and fixed the tests.

morozov · 2019-05-08T03:01:30Z

Thank you 🚢

jwage · 2019-05-08T03:20:20Z

Cool! Thanks!

Improve consistency of exception message formatting.

jwage added WIP Improvement labels Apr 30, 2019

jwage added this to the 3.0.0 milestone Apr 30, 2019

jwage marked this pull request as ready for review April 30, 2019 02:25

jwage force-pushed the improve-exception-messages branch 2 times, most recently from ed00719 to 012c657 Compare April 30, 2019 02:45

jwage requested a review from morozov April 30, 2019 03:19

greg0ire reviewed Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/DBALException.php Outdated Show resolved Hide resolved

morozov reviewed Apr 30, 2019

View reviewed changes

jwage force-pushed the improve-exception-messages branch from 012c657 to 63f36a2 Compare April 30, 2019 20:23

jwage commented Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/DBALException.php Outdated Show resolved Hide resolved

jwage commented Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php Outdated Show resolved Hide resolved

greg0ire reviewed Apr 30, 2019

View reviewed changes

lib/Doctrine/DBAL/DBALException.php Outdated Show resolved Hide resolved

jwage force-pushed the improve-exception-messages branch 3 times, most recently from 4c23709 to 6eba870 Compare April 30, 2019 22:17

morozov reviewed Apr 30, 2019

View reviewed changes

jwage force-pushed the improve-exception-messages branch from 6eba870 to 0ac207b Compare April 30, 2019 23:26

morozov reviewed May 1, 2019

View reviewed changes

lib/Doctrine/DBAL/Exception/MissingSQLType.php Outdated Show resolved Hide resolved

jwage force-pushed the improve-exception-messages branch from 0ac207b to 5ba53f9 Compare May 1, 2019 01:24

morozov reviewed May 1, 2019

View reviewed changes

jwage force-pushed the improve-exception-messages branch 2 times, most recently from 1858e72 to 2bbc38e Compare May 1, 2019 19:39

jwage commented May 1, 2019

View reviewed changes

jwage force-pushed the improve-exception-messages branch from 2bbc38e to 623623a Compare May 1, 2019 19:41

jwage commented May 1, 2019

View reviewed changes

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php Outdated Show resolved Hide resolved

jwage commented May 1, 2019

View reviewed changes

jwage force-pushed the improve-exception-messages branch 2 times, most recently from 46809de to 13820a6 Compare May 1, 2019 20:23

morozov force-pushed the develop branch from e46c09b to 5ee9443 Compare May 6, 2019 04:48

jwage force-pushed the improve-exception-messages branch from 3124f44 to 10b09e1 Compare May 7, 2019 01:11

morozov reviewed May 7, 2019

View reviewed changes

Improve consistency of exception message formatting.

67ebcf6

jwage force-pushed the improve-exception-messages branch from 10b09e1 to 67ebcf6 Compare May 7, 2019 21:07

morozov approved these changes May 8, 2019

View reviewed changes

morozov merged commit c7cb259 into doctrine:develop May 8, 2019

morozov self-assigned this May 8, 2019

morozov added a commit that referenced this pull request May 23, 2019

Merge pull request #3536 from jwage/improve-exception-messages

89766f8

Improve consistency of exception message formatting.

morozov removed the WIP label May 23, 2019

jwage added the Error Handling label May 30, 2019

morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

e9379e4

Improve consistency of exception message formatting.

morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

049fa7d

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 13, 2019

Merge pull request #3536 from jwage/improve-exception-messages

aac30ec

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 27, 2019

Merge pull request #3536 from jwage/improve-exception-messages

b6ab8df

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 27, 2019

Merge pull request #3536 from jwage/improve-exception-messages

a91b18c

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Jun 27, 2019

Merge pull request #3536 from jwage/improve-exception-messages

6f209f7

Improve consistency of exception message formatting.

morozov added a commit to morozov/dbal that referenced this pull request Aug 26, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

3e782c7

Improve consistency of exception message formatting.

morozov added a commit to morozov/dbal that referenced this pull request Aug 27, 2019

Merge pull request doctrine#3536 from jwage/improve-exception-messages

c36f826

Improve consistency of exception message formatting.

morozov added a commit that referenced this pull request Nov 2, 2019

Merge pull request #3536 from jwage/improve-exception-messages

5193c97

Improve consistency of exception message formatting.

morozov mentioned this pull request Apr 17, 2020

allow using multiple connections for CLI commands #3956

Merged

morozov mentioned this pull request May 4, 2020

Segregated support of unique index and unique constraint #3980

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve consistency of exception message formatting. #3536

Improve consistency of exception message formatting. #3536

jwage commented Apr 30, 2019 •

edited

Loading

morozov commented Apr 30, 2019

jwage May 1, 2019

morozov May 3, 2019

jwage May 1, 2019

morozov May 1, 2019

morozov commented May 6, 2019

jwage commented May 7, 2019

morozov May 7, 2019

morozov May 7, 2019 •

edited

Loading

morozov commented May 7, 2019

jwage commented May 7, 2019

morozov commented May 8, 2019

jwage commented May 8, 2019

	if (! is_numeric($shard['id']) \|\| $shard['id'] < 1) {
	throw new InvalidArgumentException('Shard Id has to be a non-negative number.');
	}

	$sql = 'SELECT member_id as id,
	distribution_name as distribution_key,
	CAST(range_low AS CHAR) AS rangeLow,
	CAST(range_high AS CHAR) AS rangeHigh
	FROM sys.federation_member_distributions d
	INNER JOIN sys.federations f ON f.federation_id = d.federation_id
	WHERE f.name = ' . $this->conn->quote($this->federationName);

Improve consistency of exception message formatting. #3536

Improve consistency of exception message formatting. #3536

Conversation

jwage commented Apr 30, 2019 • edited Loading

Summary

TODO

morozov commented Apr 30, 2019

jwage May 1, 2019

Choose a reason for hiding this comment

morozov May 3, 2019

Choose a reason for hiding this comment

jwage May 1, 2019

Choose a reason for hiding this comment

morozov May 1, 2019

Choose a reason for hiding this comment

morozov commented May 6, 2019

jwage commented May 7, 2019

morozov May 7, 2019

Choose a reason for hiding this comment

morozov May 7, 2019 • edited Loading

Choose a reason for hiding this comment

morozov commented May 7, 2019

jwage commented May 7, 2019

morozov commented May 8, 2019

jwage commented May 8, 2019

jwage commented Apr 30, 2019 •

edited

Loading

morozov May 7, 2019 •

edited

Loading