search

kukuminer / ta-app

Online application and selection platform for teaching assistants
0 stars 0 forks source link

Auth timeout #55

Closed kukuminer closed 9 months ago

kukuminer commented 10 months ago

Replace all api calls with a wrapped version which will navigate the user to the error page if their session times out (403 server response). It also allows other responses if the server returns other statuses. Does not navigate user if server responds with a 500 response. (or any other response at the moment).

kukuminer commented 9 months ago

Now refreshes the page if it receives a 403 error. Any other error is thrown back to the caller. All requests should be wrapped in try-catch from now on. I will start working on this, but I am also considering making it so it only throws on POST requests. Since GET requests are not usually triggered by a user action, there is no action to undo or prevent like with POST. @jonatanschroeder what do you think?

Also made it so that the ratings will now undo the change if the POST fails with non-403 error (as a 403 will refresh the page)

Should be ready to merge now.

jonatanschroeder commented 9 months ago

Now refreshes the page if it receives a 403 error. Any other error is thrown back to the caller. All requests should be wrapped in try-catch from now on. I will start working on this, but I am also considering making it so it only throws on POST requests. Since GET requests are not usually triggered by a user action, there is no action to undo or prevent like with POST. @jonatanschroeder what do you think?

GET requests don't typically need to be undone, but an error typically means an issue that the user needs to take into account (especially a 403). So keep it for all of them. We don't want silent errors.

Also made it so that the ratings will now undo the change if the POST fails with non-403 error (as a 403 will refresh the page)

That's great.

Should be ready to merge now.

There is a merge conflict, though...

kukuminer commented 9 months ago

We don't want silent errors.

Yes, it will still display the alert to the user. By throw to caller, I meant that the wget method from requestWrapper will throw the error back to the caller or not. The method wget has a try/catch, which displays the alert on catch. At the moment, it also throws an error on catch. If it throws, it means that all callers of the method will require a try/catch. If it doesn't throw, then callers don't require try/catch (since it won't throw the error in the code), but will still display the alert if something goes wrong since it has a try/catch internally that does so.

Merge conflict resolved btw.