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

Slim Error Händlers Verbesserung #269

Closed
ferishili opened this issue May 6, 2021 · 11 comments
Closed

Slim Error Händlers Verbesserung #269

ferishili opened this issue May 6, 2021 · 11 comments
Assignees
Labels
enhancement Merged The issue has been merged and it is ready for testing/deploying
Milestone

Comments

@ferishili
Copy link
Collaborator

No description provided.

@ferishili ferishili added this to the >=2.64 milestone May 6, 2021
@ferishili ferishili self-assigned this May 6, 2021
@crosscodr
Copy link
Contributor

Hi @ferishili, I don't know what you mean with this issue, but since we installed v2.63 we see a lot of Errors like this:

2021/06/09 14:12:31 [error] 2328869#2328869: *77462881 FastCGI sent in stderr:
 "PHP message: InvalidArgumentException: Invalid HTTP status code in /srv/studip_svn/4.6/public/plugins_packages/elan-ev/MeetingPlugin/vendor/slim/slim/Slim/Http/Response.php:227
Stack trace:
#0 /srv/studip_svn/4.6/public/plugins_packages/elan-ev/MeetingPlugin/vendor/slim/slim/Slim/Http/Response.php(191): Slim\Http\Response->filterStatus('23000')
#1 /srv/studip_svn/4.6/public/plugins_packages/elan-ev/MeetingPlugin/lib/Errors/ExceptionHandler.php(75): Slim\Http\Response->withStatus('23000')
#2 [internal function]: Meetings\Errors\ExceptionHandler->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Meetings\Errors\Error))
#3 /srv/studip_svn/4.6/public/plugins_packages/elan-ev/MeetingPlugin/vendor/slim/slim/Slim/App.php(701): call_user_func_array(Object(Meetings\Errors\ExceptionHandler), Array)
#4 /srv/studip_svn/4.6/public/plugins_packages/elan-ev/MeetingPlugin/vendor/slim/slim/Slim/App.php(394): Slim\App->handleException(Object(Meetings\Errors\Error), Object(Slim\Http\Request), Ob" while reading response header from upstream,
 client: 130.75.xxx.xxx server: studip.uni-hannover.de,
 request: "GET /plugins.php/meetingplugin/api/rooms/join/<course_id>/<room id> HTTP/2.0",
 referrer: "https://studip.uni-hannover.de/plugins.php/meetingplugin/index?cid=<course id>"

Is this the issue you mean?

@ferishili ferishili added this to To Do in Priority list Aug 19, 2021
@ferishili
Copy link
Collaborator Author

ferishili commented Aug 19, 2021

Hi @crosscodr, thanks for your info.
I have prepared some improvements regarding to this issue, is it possible for you to test a beta version.

@crosscodr
Copy link
Contributor

Hi @ferishili, I can test a beta version. When will it come up? Do you use the master branch for this?

@ferishili
Copy link
Collaborator Author

I will merge it into beta_test branch in an hour and let you know. thanks again.

@ferishili ferishili removed this from To Do in Priority list Aug 19, 2021
@ferishili
Copy link
Collaborator Author

@crosscodr the branch "beta_test" is updated and ready, please give it a try and see if the Errors are still like before.

@crosscodr
Copy link
Contributor

crosscodr commented Aug 19, 2021

Hi @ferishili, first thing I noticed: I couldn't build the plugin, since there is an old php-version defined in composer.json:

No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpunit/phpunit[7.1.0, ..., 7.5.20] require php ^7.1 -> your php version (7.0.0; overridden via config.platform, actual: 7.4.22) does not satisfy that requirement.
    - Root composer.json requires phpunit/phpunit ^7.1 -> satisfiable by phpunit/phpunit[7.1.0, ..., 7.5.20].

package.json should be updated also to current version of the plugin :)

@ferishili
Copy link
Collaborator Author

I am not able to reproduce the problems you mentioned. What do you suggest?

@crosscodr
Copy link
Contributor

I think I just have a newer version of php-unit installed, which is incompatible with php7.0, which is demanded in composer.json:

  4     "config": {
  5         "platform": {
  6             "php": "7.0.0"
  7         }
  8     },

Php 7.0 is unsupported: (see https://www.php.net/supported-versions.php).
I would suggest to increase php version to at least 7.4 (without .0).

@ferishili
Copy link
Collaborator Author

I am affraid it is currently not possible because it made a huge problem before for those Unis that run php < 7.0.1. That is the reason we came up with a minimum php 7 platform!
For the purpose of this beta testing and if it is possible for you, please go ahead and change it locally and test it.
Thanks in advance

@ferishili ferishili added the ReadyToMerge The Issues is ready to be tested label Aug 20, 2021
@crosscodr
Copy link
Contributor

crosscodr commented Aug 20, 2021

Hi @ferishili,
okay, I will change it locally. Eventually the php-unit tests should be made compatible to a supported php-version.
However, I tested the beta-branch and wondered about this new feature: default rooms.
When I upgraded the plugin, none of the existing meeting rooms of a course was marked as "default room".
Can this be ignored?

Besides that, I only saw this error in the error-log:
PHP message: PHP Warning: The use statement with non-compound name 'MeetingPlugin' has no effect in /srv/studip/public/plugins_packages/elan-ev/MeetingPlugin/app/controllers/room.php on line 9

@ferishili
Copy link
Collaborator Author

Thanks for the info:
FYI: The default room feature was the last commit that has been merged into master via #292. I hope that it did not make any problem!
The concept is that if a course has multiple rooms, it is up to lecturers to decide which room is default, however if there is only one room it will be selected as default automatically.

ferishili added a commit that referenced this issue Aug 20, 2021
@ferishili ferishili added Merged The issue has been merged and it is ready for testing/deploying and removed ReadyToMerge The Issues is ready to be tested labels Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Merged The issue has been merged and it is ready for testing/deploying
Projects
None yet
Development

No branches or pull requests

2 participants