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

Session: keep exceptions chain on session_start #240

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

forrest79
Copy link
Contributor

  • new feature
  • BC break? no

There was an issue #208 - phpredis session driver was raising a notice, when session lock can't be obtained. Nette Session was correctly converting this notice to an exception. The strange was, that even the notice was raised, session_start returned true (it was phpredis problem).

Now, the new production version of phpredis is out, and the behavior is a little bit different. If a lock can't be obtained, instead of notice, the warning (instead of a notice) is raised and the session_start returned correctly false.

The problem is, I need to know the reason why the session wasn't started (when redis is death, it's a problem, when session lock can't be obtained, it's probably OK - user is probably doing many parallel requests). Nette\Utils\Callback::invokeSafe around the session_start method correctly converts warnings to exceptions, but now, there are two exceptions:

  • the first is from the phpredis - something like Failed to acquire session lock
  • the second is from the PHP itself Failed to read session data: redis (path: tcp://server:6379?timeout=3&database=1&prefix=session:v1:)

And the first one is overwritten with the second one, that is thrown to the app. But I need to know also the first one :-)

So my first idea is to set correct previous exception property and in the app, I have the complete exceptions chain, that I can read and get the correct session problem.

@dg
Copy link
Member

dg commented Sep 18, 2024

thanks

@dg dg merged commit afdd2e1 into nette:master Sep 18, 2024
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

Successfully merging this pull request may close these issues.

2 participants