Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rate limit error handling. #2

Merged
merged 5 commits into from
Jan 23, 2019
Merged

Conversation

nhorvath
Copy link
Contributor

Gracefully handles rate limiting in the Photos API.
gphotosuploader/gphotos-uploader-cli#22

lib-gphotos/client.go Outdated Show resolved Hide resolved
lib-gphotos/client.go Outdated Show resolved Hide resolved
continue
} else if retryCount < 3 {
log.Printf("Error during upload, sleeping for 10 seconds before retrying...")
time.Sleep(10 * time.Second)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need this second else if. I think we could change the for retry != false { for something like:

for retry != false && retryCount < 3 {

and remove this code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want to handle errors other than rate limit differently than other errors so I think this should still stay like this.

@nhorvath
Copy link
Contributor Author

nhorvath commented Dec 27, 2018

@pacoorozco I got this for the first time after making these changes. Could it be caused by anything I did in the last commit?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x70cd9d]

goroutine 12 [running]:
github.com/nmrshll/google-photos-api-client-go/lib-gphotos.(*Client).UploadFile(0xc0001d20e8, 0xc000560000, 0xa3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/nick/go/src/github.com/nmrshll/google-photos-api-client-go/lib-gphotos/client.go:122 +0x3ed
github.com/nmrshll/gphotos-uploader-cli/upload.(*FileUpload).upload(0xc0001d20c0, 0xc000405fa8, 0x1)
        /home/nick/go/src/github.com/nmrshll/gphotos-uploader-cli/upload/fileUpload.go:51 +0x9e
github.com/nmrshll/gphotos-uploader-cli/upload.StartFileUploadWorker.func1(0xc00014a048)
        /home/nick/go/src/github.com/nmrshll/gphotos-uploader-cli/upload/fileUpload.go:31 +0x75
created by github.com/nmrshll/gphotos-uploader-cli/upload.StartFileUploadWorker
        /home/nick/go/src/github.com/nmrshll/gphotos-uploader-cli/upload/fileUpload.go:29 +0x87
exit status 2

edit: yes it's this line

			if batchResponse.ServerResponse.HTTPStatusCode == 429 {

@nhorvath
Copy link
Contributor Author

@pacoorozco Ah I remember now. You don't have access to the response if error is != nil. That's why I did it this way.
From google's docs (emphasis mine):
Do executes the "photoslibrary.mediaItems.batchCreate" call. Exactly one of *BatchCreateMediaItemsResponse or error will be non-nil. Any non-2xx status code is an error. Response headers are in either *BatchCreateMediaItemsResponse.ServerResponse.Header or (if a response was returned at all) in error.(*googleapi.Error).Header. Use googleapi.IsNotModified to check whether the returned error was because http.StatusNotModified was returned.

@pacoorozco
Copy link

pacoorozco commented Dec 27, 2018

Yes @nhorvath, you are absolutely right! I don't think Google's doing things easy to deal with errors, but your new code it's right.... Sorry for bother you :-D +1

@nmrshll
Copy link
Owner

nmrshll commented Jan 23, 2019

Looks good to merge to me !

@nmrshll nmrshll merged commit 94a9969 into nmrshll:master Jan 23, 2019
pdecat pushed a commit to pdecat/google-photos-api-client-go that referenced this pull request Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants