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

api v4 won't return data if there're errors #1956

Closed
janat08 opened this issue Apr 21, 2019 · 4 comments
Closed

api v4 won't return data if there're errors #1956

janat08 opened this issue Apr 21, 2019 · 4 comments
Labels
meta Related to Refined GitHub itself

Comments

@janat08
Copy link
Contributor

janat08 commented Apr 21, 2019

I'm querying about organizations, and those may restrict third-party api access regarding them, and then I'm querying about users as I don't know if repository owner is actually an organization or a user, at which point one of those will definitely error.

Given that with graphql you might be asking for plenty of data/fields in a single query it is wrong to throw errors and not return data. I suspect that even rate limiting/depth might be field dependent.

@janat08 janat08 added the bug label Apr 21, 2019
@fregante
Copy link
Member

it is wrong to throw errors and not return data

Indeed, that's one of the drawbacks of throwing errors, as I mentioned here: #1679

I don't know if repository owner is actually an organization or a user, at which point one of those will definitely error.

Can you show the query you're talking about specifically? Maybe we can see if it can be done in another way.

I suspect that even rate limiting/depth might be field dependent.

We can't handle "partial API calls", it's just too much work. If we need some information and we can't get it because of rate limiting, I'm ok with the feature breaking.

@janat08
Copy link
Contributor Author

janat08 commented Apr 21, 2019

I very much doubt that there's a field for checking out if organization gives aways info about itself to third-party apps (besides that I have checked organization fields). It not doing so will manifest as error.

		{
			repository(owner: "${ownerName}", name: "${repoName}") {
				projects(states: [OPEN, CLOSED]){
					totalCount
				}
				issue(number: ${number}){
    				viewerCanUpdate
    			  	projectCards{
    			    	totalCount
    				}
    			}
			}
			organization(login: "${ownerName}"){
			    projects(states: [OPEN, CLOSED]){
        			totalCount
    			}
			}
			user(login: "${ownerName}"){
				projects(states: [OPEN, CLOSED]){
					totalCount
				}
			}
			
		}

@janat08
Copy link
Contributor Author

janat08 commented Apr 21, 2019

There's an option of simplifying feature im working on to be obtuse about when it applies itself.

@fregante fregante added meta Related to Refined GitHub itself and removed bug labels Apr 21, 2019
@fregante
Copy link
Member

Making this change would bring the error checking into each feature and make the code more complex. I think we're good for now since it's not needed, until we really can't do without it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Related to Refined GitHub itself
Development

No branches or pull requests

2 participants