[DoctrineBundle] Proxy autoloader should not throw exception #4168

Merged
merged 2 commits into from May 15, 2012

4 participants

@robocoder

Also change 'require' to non-fatal '@include' in the event no file is generated.

@robocoder robocoder Autoloader should not throw exception because PHP will continue to ca…
…ll other registered autoloaders.

Change 'require' to non-fatal '@include' in the event no file is generated.  Let PHP handle the undefined class, natively.
1ed8b72
@stof
Symfony member

The goal of the exception was to make debugging easier. And all Symfony2 autoloaders are using require

@robocoder

I changed the include back to a require.

Whether or not the exception makes debugging easier is debatable. But throwing an exception from an autoloader is both unconventional and unexpected given that (1) exceptions are propagated while php calls other registered autoloaders, and (2) php will throw a fatal error where the usage actually occurs if the class doesn't exist.

@fabpot
Symfony member

ping @beberlei

@beberlei

Its tricky, the message does try to give some additional information - but later autoloaders could handle this issue anyways. I guess the PR makes sense as users have absolutely no control over this autoloader and it should therefore behave less strictly.

@fabpot fabpot added a commit that referenced this pull request May 15, 2012
@fabpot fabpot merged branch robocoder/autoloader (PR #4168)
Commits
-------

b2afd9f use require instead of include
1ed8b72 Autoloader should not throw exception because PHP will continue to call other registered autoloaders.

Discussion
----------

[DoctrineBundle] Proxy autoloader should not throw exception

Also change 'require' to non-fatal '@include' in the event no file is generated.

---------------------------------------------------------------------------

by stof at 2012-05-01T06:13:34Z

The goal of the exception was to make debugging easier. And all Symfony2 autoloaders are using ``require``

---------------------------------------------------------------------------

by robocoder at 2012-05-01T16:09:04Z

I changed the include back to a require.

Whether or not the exception makes debugging easier is debatable.  But throwing an exception from an autoloader is both unconventional and unexpected given that (1) exceptions are propagated while php calls other registered autoloaders, and (2) php will throw a fatal error where the usage actually occurs if the class doesn't exist.

---------------------------------------------------------------------------

by fabpot at 2012-05-15T06:01:11Z

ping @beberlei

---------------------------------------------------------------------------

by beberlei at 2012-05-15T10:20:06Z

Its tricky, the message does try to give some additional information - but later autoloaders could handle this issue anyways. I guess the PR makes sense as users have absolutely no control over this autoloader and it should therefore behave less strictly.
ec45169
@fabpot fabpot merged commit b2afd9f into symfony:2.0 May 15, 2012
@beberlei

@robocoder Can you make a PR on github.com/doctrine/DoctrineBundle as well?

@mmucklo mmucklo pushed a commit that referenced this pull request May 23, 2013
@fabpot fabpot merged branch robocoder/autoloader (PR #4168)
Commits
-------

b2afd9f use require instead of include
1ed8b72 Autoloader should not throw exception because PHP will continue to call other registered autoloaders.

Discussion
----------

[DoctrineBundle] Proxy autoloader should not throw exception

Also change 'require' to non-fatal '@include' in the event no file is generated.

---------------------------------------------------------------------------

by stof at 2012-05-01T06:13:34Z

The goal of the exception was to make debugging easier. And all Symfony2 autoloaders are using ``require``

---------------------------------------------------------------------------

by robocoder at 2012-05-01T16:09:04Z

I changed the include back to a require.

Whether or not the exception makes debugging easier is debatable.  But throwing an exception from an autoloader is both unconventional and unexpected given that (1) exceptions are propagated while php calls other registered autoloaders, and (2) php will throw a fatal error where the usage actually occurs if the class doesn't exist.

---------------------------------------------------------------------------

by fabpot at 2012-05-15T06:01:11Z

ping @beberlei

---------------------------------------------------------------------------

by beberlei at 2012-05-15T10:20:06Z

Its tricky, the message does try to give some additional information - but later autoloaders could handle this issue anyways. I guess the PR makes sense as users have absolutely no control over this autoloader and it should therefore behave less strictly.
563cc27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment