jpillora / node-edit-google-spreadsheet

A simple API for editing Google Spreadsheets
304 stars 101 forks source link

Request does not follow 302 redirects from Google #52

Closed TimJohns closed 9 years ago

TimJohns commented 9 years ago

Hello and thanks to Jaime Pillora and the other contributors for the great module!

I ran into an issue recently where Google is frequently returning a 302, and the request code in the module would return the response body as an error to my code.

For instance, after adding cells, when I would call .send(), I would see 'Sending updates...' from the debug logging, but then my callback would be called with an HTML body that contained common 'Moved Temporarily' text. I dug in a little bit, and Google was responding to a POSTs to /feeds/cells/1-QPKWX_kQVs3LmHY3djpDZ0qVjTPUHMgs-G4NLQ0ksc/od6/private/full/batch with a 302 redirect with location header of /feeds/cells/1-QPKWX_kQVs3LmHY3djpDZ0qVjTPUHMgs-G4NLQ0ksc/od6/private/full/batch?lsrp=1

The gist is that Google is apparently redirecting to add lsrp=1 as a query string, but we are treating the redirect like an error.

I locally modified the Spreadsheet.prototype.request in lib/index.js to follow the redirect, and now it works just fine.

I'm a little suspicious of my fix only because it happens very frequently for me, yet I didn't see anyone else having this same issue, but it makes sense to me and I'm happy to create a PR as soon as I figure out how to create a PR... I Googled around for information on the lsrp query string parameter, but came up empty, and I'm wondering if anyone else knows what that's for as well (perhaps, for example, in addition the redirect fix, it could be appropriate to append that query string to the URL in the original POST).

Has anyone else seen very common redirects from Google when POSTing batches of cell updates like that?

I also see a surprisingly high number of 500's from Google as well, but those seem to be more transient and much less reproducible. Separate issue, but I'm curious if anyone else is seeing the same thing.

richmarr commented 9 years ago

Thanks @TimJohns.

TL;DR this is a decent solution to a real problem. +1 to accepting the PR.

I've been seeing these 302s too. My solution was to append &ndplr=1 to the request URL in order to instruct Google not to redirect you, but your solution seems cleaner as it doesn't require inside knowledge of potentially changeable query parameters. It does seem like a known issue, in my case occurring repeatably on the second batch of updates pushed to a single sheet. This was the source of my hack in case that's of any use: http://blog.forret.com/2011/07/google-docs-infamous-moved-temporarily-error-fixed/

jpillora commented 9 years ago

Fixed by #53