brownplt / code.pyret.org

Website for serving Pyret to folks.
Other
24 stars 44 forks source link

GDrive share doesn't load properly #255

Closed sorawee closed 6 years ago

sorawee commented 6 years ago

This is a pretty long GIF, but it describes what happens best.

https://giphy.com/gifs/xT0xeyfEG85YhRTy24/fullscreen

That is, a shared file (not the original code file) is occasionally loaded as a blank file in #editor mode. But it is always loaded as a blank file in #share mode. The permission of the file is correct. The console doesn't have any error. This happens across browsers (Firefox 57 and Vivaldi / Chrome 62)

The problematic URL: https://code.pyret.org/editor#share=0Bxr4FfLa3goORTBQSmRfOEk3eHM

Note that this link is the stencil file for the interp assignment that we gave to students two months ago, so it definitely used to work.

sorawee commented 6 years ago

It seems the GIF got trimmed. Here's the full version:

https://www.dropbox.com/s/xlflw36456ukomj/Peek%202017-11-25%2018-15.mp4

jpolitz commented 6 years ago

Ugh. This is almost certainly more horrible-ness with the way Drive requires that you don't include credentials for requests to public files.

This is failing only for #share links for logged in users (if you visit the link in an incognito tab, it'll work fine).

Thanks.

On Sat, Nov 25, 2017 at 3:50 PM, sorawee notifications@github.com wrote:

It seems the GIF got trimmed. Here's the full version:

https://www.dropbox.com/s/xlflw36456ukomj/Peek%202017-11-25%2018-15.mp4

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/brownplt/code.pyret.org/issues/255#issuecomment-346973892, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHUU6qCUu8OYIiCNBZw701XlESOhHFSks5s6KepgaJpZM4QqmH4 .

jpolitz commented 6 years ago

Yeah this is in direct conflict with a fix I made earlier this fall and pushed out recently.

Here's the crux of the issue:

  1. The Drive API forces you to set the authorization token to null before sending requests for public on the web files, as documented by the Drive team here (https://plus.google.com/u/0/+JoeGibbsPolitz/posts/7xLaiAMUP8q?cfem=1https://plus.google.com/u/0/+JoeGibbsPolitz/posts/7xLaiAMUP8q?cfem=1)
  2. As far as I can tell, the authorization token is a global mutable property of the Google Drive API; you manipulate it with a method called setToken. This property is read inside the library whenever you execute a request to determine which credential to send with the request.
  3. When you execute a request, the result is a promise for when the request completes (whether it succeeds or fails).
  4. There is no event for the request being sent, which might help tell you when to set the authorization token back.
  5. Deep in the guts of the gapi system, it appears (screenshot below) that there is a setTimeout wrapped around the sending of the request.

image

As a result of 1-5, the two obvious ideas don't work:

  1. If you set the credential back immediately after calling execute, this issue results, where the credential is set immediately to null before the request even goes out.
  2. If you set the credential back when the request completes, then any requests that go out in the meantime don't have the access token correctly set. This causes huge issues where changing the order of my-gdrive and shared-gdrive imports has bizarre effects, because CPO will make parallel requests to Drive to shorten wait times.

Now I'm thinking that using a setTimeout wrapped around a thunk that sets the credential back might have the right properties of updating it on the right turn, even with multiple requests in parallel. I'll give that a shot, but I wanted to write this all down while it was fresh in my mind; this issue has bounced around on email and slack but not had a clear writeup on Github before.