Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improveException should suggest installing missing extensions #221

Closed
JanTvrdik opened this issue Sep 8, 2016 · 13 comments
Closed

improveException should suggest installing missing extensions #221

JanTvrdik opened this issue Sep 8, 2016 · 13 comments

Comments

@JanTvrdik
Copy link
Contributor

JanTvrdik commented Sep 8, 2016

For example Call to undefined function Nette\Mail\finfo_buffer, should be improved to

Call to undefined function Nette\Mail\finfo_buffer, you need to enable finfo extension in php.ini

We obviously can't have a large array with all possible php functions, but having a simple array mapping function prefix to extension name should work well enough for most use-cases.


@dg If you think this would be useful, you may tag this issue as easy pick to encourage contributions.

@adaamz
Copy link

adaamz commented Sep 8, 2016

+multibyte + disabled move_uploaded_file function etc.

@kravcik
Copy link

kravcik commented Sep 8, 2016

I guess everybody should run checker before start working. So this feature will be obsolete.

@JanTvrdik
Copy link
Contributor Author

@kravcik no, requirements checker solves different issue and does not help people who use Tracy without other Nette libraries

@dg
Copy link
Member

dg commented Sep 8, 2016

IMHO the most confusing is namespace Nette\Mail\, it can be removed using \finfo_buffer or use function finfo_buffer. Or there can be check in code.

The problem only applies to extension fileinfo in nette/http and nette/mail and some of these extensions in Latte and utils. Some of them are checked in code, some checks can be added. I am 👎 to solve it in Tracy.

@JanTvrdik
Copy link
Contributor Author

The problem only applies to extension

I did not intended this feature for PHP extensions used by Nette but for all commonly missing PHP extensions such as json, curl, mbstring, openssl, ...

@dg
Copy link
Member

dg commented Sep 8, 2016

As a general solution it probably makes sense.

BTW https://bugs.php.net/bug.php?id=73049

@Majkl578
Copy link
Contributor

Majkl578 commented Sep 8, 2016

Probably good idea in general, but the message should be more generic (i.e. not mention php.ini) and work not only with extension-related functions (i.e. new/removed core function like set_magic_quotes_runtime()).

BTW https://bugs.php.net/bug.php?id=73049

I believe the message is correct (although confusing), since the lookup is namespace-local first, then global. Same for constants. Maybe it could mention both variants in th message but either is partially correct.

@JanTvrdik
Copy link
Contributor Author

not mention php.ini

Why not mention php.ini? Can you post an example of how such message should look like according to you?

work not only with extension-related functions

That seems out-of scope.

I believe the message is correct (although confusing)

Have you read the bug report? Because that's exatly what is states.

@Majkl578
Copy link
Contributor

Majkl578 commented Sep 8, 2016

Can you post an example of how such message should look like according to you?

Call to undefined function Nette\Mail\finfo_buffer or \finfo_buffer, ensure that finfo extension is enabled and loaded.

or maybe even like this to avoid namespace confusion:
Call to undefined function finfo_buffer from namespace Nette\Mail, ensure that finfo extension is enabled and loaded.

Have you read the bug report? Because that's exatly what is states.

Yes and it does not. Quoting:

The better message should be "Call to undefined function json_encode()".

It drops the information about namespace entirely (so if you were using e.g. namespaced function like from GuzzleHttp or libsodium, you would have hard time to figuring that out, because function is not prefixed by extension name - situation depends on actual context / current namespace).

@dg
Copy link
Member

dg commented Sep 8, 2016

would have hard time to figuring that out

There is file name and line number.

@JanTvrdik
Copy link
Contributor Author

Call to undefined function Nette\Mail\finfo_buffer or \finfo_buffer, ensure that finfo extension is enabled and loaded.

But we can know for sure (extension_loaded) whether the extension is loaded. There is no need for the user to check whether the extension is loaded – we know it is not.

Yes and it does not. Quoting:

You have said „I believe the message is correct (although confusing)“ and @dg has said „the error message (…) is (…) confusing.“ and „both error messages are correct“

@dg
Copy link
Member

dg commented Sep 8, 2016

For message Call to undefined function (namespace\)?(prefix)_.* when prefix is one of the listed prefixes and correspoding extension is not loaded, ideal message is Call to undefined function finfo_buffer, you need to enable finfo extension, i.e. without namespace. I am not sure about php.ini.

@dg
Copy link
Member

dg commented Apr 6, 2018

@JanTvrdik Will you try to implement it?

@dg dg closed this as completed Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants