Skip to content
This repository was archived by the owner on Feb 5, 2024. It is now read-only.

Conversation

@manupsunny
Copy link

As there are no/limited error handling mechanisms available for the end user now, returning exception to the callback method on getting an invalid json back from Google service.

@manupsunny manupsunny closed this Nov 9, 2016
@manupsunny manupsunny reopened this Nov 9, 2016
@EirikBirkeland
Copy link

manupsunny Please check out this one too which I did without first seeing your change:
#131

My version returns a status code such as 400 and 404 as soon as the xhr request fails.

I think our versions can be easily combined, since it may still be possible for the xhr request to succeed while the user still experiencing the same error later on. I will look into this later.

Also I think it would be nice to have standard messages for each HTTP status code based on what's usually the problem -- for example, an unpublished spreadsheet should result in a 403 error, while 404 is obviously an invalid URL, and 400 is some kind of less specific problem with the xhr request sent to the server (like header corruption I believe.

@EirikBirkeland
Copy link

EirikBirkeland commented Dec 6, 2016

And further in regard to HTTP error codes, I think I would actually prefer to customize these myself, but perhaps we could return a detailed error object, like this:

{
   code: 403,
   message: "403 Forbidden – Your request to view the public version of the spreadsheet was rejected by Google's servers. Please ensure that you have made your spreadsheet public."
}

Then the user would just deal with the first err parameter just like we both had in mind:

const mySheet = Tabletop.init(myUrl)

mySheet.fetch(err, res) {
  if (err) handleError(err)
  // do stuff with result here if no error
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants