gctools-outilsgc / gccollab

Other
23 stars 7 forks source link

Fatal Error Exception on Successful comment post #123

Closed sdkibb closed 7 years ago

sdkibb commented 7 years ago

This was happening in GCconnex so please speak with @Phanoix and bring over the fix (if available) from GCconnex.

I posted a comment and received the Fatal Error screen of death (slightly redundant there). Upon going back to the page I could see that my comment was successfully posted.

This should be given high priority.

markwooff commented 7 years ago

Was discussing this issue with Derek today, he was able to provide a screenshot as well:

image

sdkibb commented 7 years ago

Yeah, known issue with GCc team.

ToferC commented 7 years ago

Hi all - this is happening with GCconnex, but I think it's a different issue related to server load. Regardless, the devs on both teams need to keep in touch. We need to prioritize fixing this on GCcollab ASAP. Thanks all. @sdkibb @markwooff @Phanoix

Phanoix commented 7 years ago

I'll take a look at the collab logs, maybe they'll have more clues than the connex ones if these are in any way related.

Phanoix commented 7 years ago

That particular one is because it was trying to send a notification email that didn't have a subject.

markwooff commented 7 years ago

It seems to be stemming from the cp_notifications plugin, an exception is being thrown when there is no subject used for a notification email:

Exception #1493835904: exception 'NotificationException' with message 'NotificationException:MissingParameter' in /var/www/html/gccollab/mod/phpmailer/start.php:172 Stack trace:

0 /var/www/html/gccollab/mod/cp_notifications/start.php(807)

Have you seen this on Connex before? Wonder if we should be setting a standard 'default subject' if there is none.

sdkibb commented 7 years ago

We should explore with a mindset that they are related given the similar error and successful posting result. Could be server load is not the issue. Collab is way underutilized. I'll test more tonight on dev.

Phanoix commented 7 years ago

We probably get are some of those on connex.

markwooff commented 7 years ago

Just looking at the two repos & it appears the cp_notifications module on Collab is out of date already. Many changes done to this module on Connex. Mainly from https://github.com/gctools-outilsgc/gcconnex/commit/9c5f6efaa8972386b3c80933489e6be082120120

Phanoix commented 7 years ago

Yeah, looks like the connex version shouldn't have this particular problem.

markwooff commented 7 years ago

@Phanoix I know we did one recently, but do you think we could do another code pull from connex to collab? Should cover a lot of those accessibility changes done recently too.

Phanoix commented 7 years ago

Should probably merge the fix for this issue quickly first, and then get the rest up to the end of this sprint.

markwooff commented 7 years ago

I've merged the relevant notification changes from the PR https://github.com/gctools-outilsgc/gccollab/pull/124 and placed into dev. @sdkibb you will be getting lots of emails from my testing of notifications to verify the NotificationException doesn't appear again.

Will merge into prod after testing is completed.

markwooff commented 7 years ago

Fix for this exception has been pushed into prod from the latest PR https://github.com/gctools-outilsgc/gccollab/pull/125.