search

llaske / sugarizer-server

Sugarizer Server allow deployment of Sugarizer on a local server, for example on a school server.
Apache License 2.0
42 stars 92 forks source link

Server crashes on client reload #51

Closed NikhilM98 closed 5 years ago

NikhilM98 commented 5 years ago

Steps to reproduce:

  1. Start the server using npm start
  2. Login and navigate to /dashboard/activities page (From Browser).
  3. Stop the server. Start the server again. (Restart)
  4. Refresh the /dashboard/activities page. The server crashes: screenshot from 2019-03-03 18-18-49

Expected behavior:

NikhilM98 commented 5 years ago

It's working fine on other pages.

On comparing: https://github.com/llaske/sugarizer-server/blob/a292c95a7e91c140a7ad05e5844c8c86eb41c9d8/dashboard/controller/users.js#L54-L74 and

https://github.com/llaske/sugarizer-server/blob/a292c95a7e91c140a7ad05e5844c8c86eb41c9d8/dashboard/controller/activities/index.js#L34-L52

I would recommend removing return res.redirect('/dashboard/activities'); statement present in the else statement.

I removed return res.redirect('/dashboard/activities'); and the code seems to work correctly. The server does not crashes and it redirects to the login page. screenshot from 2019-03-03 18-36-03

NikhilM98 commented 5 years ago

@ashish0910 Thanks for pointing it out but I was already working on this issue and I have an open pull request. Check contributing guidilines here

Edit: My current implementation does not hold true for Journal case. It won't be able to handle cases like Error Code 15 or Error Code 16. I'll make another commit handling those cases in Journal view.

ashish0910 commented 5 years ago

Sorry @NikhilM98 I referred your issue just as an example . It is not conflicting your pull request and has change in different files . The issue which my pull request fix is quite similar to this issue but for different set of files . Your issue and pr was specifically for activity page and mine works on journal page . If you still feel objected then I can close my PR .

NikhilM98 commented 5 years ago

@ashish0910 since your implementation was similar to mine, It couldn't handle getEntries functionality properly. It was good for rendering index. The server is crashing due to an authentication error. But the app should also be able to handle other error cases.

Edit: My current implementation does not hold true for Journal case. It won't be able to handle cases like Error Code 15 or Error Code 16. I'll make another commit handling those cases in Journal view.

ashish0910 commented 5 years ago

@NikhilM98 since now you are now working on journal page also then I shall close my PR .

NikhilM98 commented 5 years ago

Fixed in #52