Adding context information when requesting invalid column type #7013
Conversation
mamazu
force-pushed
the
better_error_messages
branch
from
ed91f05 to
6c06126
Compare
| try { | ||
| $column = $this->_getPortableTableColumnDefinition($tableColumn); | ||
| } catch (UnknownColumnType $unknownTypeException) { | ||
| throw UnknownColumnType::withContext($unknownTypeException->getType(), sprintf('table %s', $table)); |
There was a problem hiding this comment.
I think this is overcomplication. Instead, we could try reusing UnsupportedTableDefinition and adding another static constructor (e.g. fromIntrospectionException(). It would accept the table name and reference the exception caught from _getPortableTableColumnDefinition() as the previous exception.
This way, we will be able to supply any introspection exception with the context (table name) w/o introducing new classes.
There was a problem hiding this comment.
Sounds like a reasonable idea but what is this UnsupportedTableDefinition class? I can't find it in the source code or the dependencies.
There was a problem hiding this comment.
Sorry for the confusion. This class is available only in 5.0.x (introduced in #6853).
Please feel free to re-introduce it in your PR (w/o the methods that exist in 5.0.x, only with yours).
Note that the 4.2.x branch accepts only bug fixes. An improvement like this needs to be targeted at 4.3.x.
There was a problem hiding this comment.
Hm thinking about this, this change might be better suited for 5.0 anyway then seeing that using a different exception would also change the type of exception thrown in the method. However, I think it still would be cool to have it in 4.3.x without any bc breaks.
How do other parts of the application handle those kinds of things. Like adding context to an exception sounds like a common enough use-case.
mamazu
force-pushed
the
better_error_messages
branch
from
6c06126 to
4a10788
Compare
Summary
Adding a way to set context on the
UnkownColumnType. However in the place where I've added it there is no easy way to also get the name of the column, so I'm just providing the table, which should also already help.