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: correct the logic to fetch transactions #467

Merged
merged 4 commits into from
Oct 29, 2024

Conversation

moreal
Copy link
Contributor

@moreal moreal commented Oct 29, 2024

This pull request fixes the logic to fetch transactions. To do it, I made some changes:

  • Validate GetTransactionsResponse manually. It validates responses like below:
    {
    	"data": {
    		"transaction": null
    	},
    	...
    }
    {
    	"data": {
    		"transaction": {
    			"ncTransactions": null
    		}
    	},
    	...
    }
    {
    	"data": {
    		"transaction": {
    			"ncTransactions": [
    				{
    					...,
    					"actions": [
    						null
    					],
    					...
    				}
    			]
    		}
    	},
    	...
    }
  • Checks "errors" field is not null because there will be errors field when there were any errors while resolving the query, according to the GraphQL spec, https://spec.graphql.org/October2021/#sec-Errors.
  • Expire cached value after 30 seconds from writing. Though it tried to validate the responses, it can cache invalid response unfortunately. So it expire the cached value after 30 seconds from writing.

@auto-assign auto-assign bot requested review from Atralupus and boscohyun October 29, 2024 10:45
@moreal moreal added bug Something isn't working mimir.worker labels Oct 29, 2024
@moreal
Copy link
Contributor Author

moreal commented Oct 29, 2024

In internal chat, I had a conversation with @boscohyun about how to handle errors field in GraphQL response because current HeadlessGQLClient's behavior doesn't follow GraphQL's spec completely.

However, we agreed that HeadlessGQLClient's current behavior is enough in current situation, so I'll merge this pull request and continue the discussion in #468.

@moreal moreal added this pull request to the merge queue Oct 29, 2024
Merged via the queue into planetarium:main with commit e5b0f8e Oct 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mimir.worker
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants