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

fix: Handle errors generated during input object thunks #123

Merged

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Jan 19, 2022

Closes #112

Also includes a fix for a hidden issue where errors were being silently generated during many of our tests but being ignored (see commit Generate Filter base args for all objects before resolving). Note that fixing this issue removes any currently generateable errors from the current thunks, however I believe we should still have support for this lest we later introduce more hidden errors.

Dependent on sourcenetwork/graphql-go#3

Todo:

  • Update go.mod once graphql-go fork PR is merged

@AndrewSisley AndrewSisley added bug Something isn't working area/schema Related to the schema system labels Jan 19, 2022
@AndrewSisley AndrewSisley self-assigned this Jan 19, 2022
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I112-input-object-thunk-error-handling branch from eab4b1a to 7abc59d Compare January 22, 2022 21:19
@codecov
Copy link

codecov bot commented Jan 22, 2022

Codecov Report

Merging #123 (826791a) into develop (303df74) will decrease coverage by 0.01%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #123      +/-   ##
===========================================
- Coverage    63.86%   63.85%   -0.02%     
===========================================
  Files           80       80              
  Lines         7328     7334       +6     
===========================================
+ Hits          4680     4683       +3     
- Misses        2163     2165       +2     
- Partials       485      486       +1     
Impacted Files Coverage Δ
query/graphql/schema/generate.go 82.77% <78.57%> (-0.37%) ⬇️

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

Just a quick question/comment on the FromAST func

query/graphql/schema/generate.go Show resolved Hide resolved
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

LGTM

Is a hidden issue, AppendType is actually returning an error for many of our unit tests however it is being ignored
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I112-input-object-thunk-error-handling branch from 7abc59d to 826791a Compare January 27, 2022 19:09
@AndrewSisley AndrewSisley merged commit d0ba8d4 into develop Jan 27, 2022
@AndrewSisley AndrewSisley deleted the sisley/fix/I112-input-object-thunk-error-handling branch January 27, 2022 19:12
@AndrewSisley
Copy link
Contributor Author

LGTM

Cheers :)

@orpheuslummis orpheuslummis mentioned this pull request Jan 29, 2022
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…k#123)

* Generate Filter base args for all objects before resolving

Is a hidden issue, AppendType is actually returning an error for many of our unit tests however it is being ignored

* Handle errors raised in input object thunks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Related to the schema system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some thunks in generate.go do not correctly handle errors
2 participants