illinois / queue

A microservice queue for holding open office hours
University of Illinois/NCSA Open Source License
82 stars 36 forks source link

Rewrite queue page with hooks #263

Closed nwalters512 closed 5 years ago

nwalters512 commented 5 years ago

~My hope is that this rewrite will help eliminate the 404s we've been seeing at random. My best guess (or, hunch, really) as to why it's happening is a race condition of some kind, which hooks are better at avoiding. We also explicitly maintain our own loading state in the component. Again, this is just a hunch, and we'll have to see if the anecdotal reports of 404s stop after this.~

Turns out my initial hunch was totally wrong. Eddie figured out how to reproduce the 404 bug:

It appears as though Safari kills socket connections when Safari isn't in the foreground. When that would happen, we'd get an error event, which (due to my bad error handling) would cause another error. That error would get picked up by React and rendered as our custom error page by Next.js. Unfortunately, our custom error page didn't work well for this use case, because it would default to showing a misleading 404 if a status code wasn't present on the error (which it wasn't, in this case).

So, this PR does a few things to address these issues:

Given the context of the original problem, would be awesome if people have iOS devices to test with.

Also fixes or enhances a few other things:

vercel[bot] commented 5 years ago

This pull request is automatically deployed with Now. To access deployments, click Details below or on the icon next to each push.

nwalters512 commented 5 years ago

Also tested this on Now with Eddie's iPad - it correctly reports that the socket is disconnected, then connecting, then connected again after the device is awoken from sleep.