Improved error message when external schema cannot be found #852
Conversation
| @@ -364,6 +364,11 @@ protected function includeExternalSchemas(\DOMDocument $dom, $srcDir) | |||
|
|
|||
| $externalSchema->parentNode->removeChild($externalSchema); | |||
|
|
|||
| $path = realpath($include); | |||
| if (!$path) { | |||
| throw new BuildException("Unable to find external schema '$path'"); | |||
There was a problem hiding this comment.
Thanks for pointing out :)
mpscholten
force-pushed
the
fix-external-schema-error
branch
from
1b29943
to
458d112
Compare
|
Looks good, thx 👍 maybe add a unit test? |
| @@ -364,8 +364,13 @@ protected function includeExternalSchemas(\DOMDocument $dom, $srcDir) | |||
|
|
|||
| $externalSchema->parentNode->removeChild($externalSchema); | |||
|
|
|||
| $path = realpath($include); | |||
| if (!$path) { | |||
There was a problem hiding this comment.
Why not just file_exists on $include? It resolves symlinks on its own.
There was a problem hiding this comment.
Should we also check is_readable?
There was a problem hiding this comment.
Doesn't is_readable without file_exists also works? is_readable basically includes file_exists
We don't have unit tests for this class at the moment. Not sure whether it's worth it to add them for this small change. |
|
I don't think its worth, but if you are incredible motivated, it won't hurt. :D Sent from my iPhone
|
|
Haha no. Just wanted to fix my php warning, and then thought this might be useful for other people to :)
|
mpscholten
force-pushed
the
fix-external-schema-error
branch
from
458d112
to
ac1f45b
Compare
|
Just added |
| @@ -364,6 +364,10 @@ protected function includeExternalSchemas(\DOMDocument $dom, $srcDir) | |||
|
|
|||
| $externalSchema->parentNode->removeChild($externalSchema); | |||
|
|
|||
| if (!is_readable($include)) { | |||
| throw new BuildException("Unable to find external schema '$include'"); | |||
There was a problem hiding this comment.
"Either does not exist or is not readable"
mpscholten
force-pushed
the
fix-external-schema-error
branch
from
ac1f45b
to
07258df
Compare
|
Ok, cool, thanks! 😃 |
Improved error message when external schema cannot be found
This improves the error message when you include an external schema which could not be found by propel. Currently you get the following error:
Now you get
Propel applies
realpathto the path given by the user. Ifrealpathcannot resolve the given path it'll returnfalse. Nowfalseis given toDOMDocument::load, which logs a php warning because of invalid input.