zino-hofmann / graphql-flutter

A GraphQL client for Flutter, bringing all the features from a modern GraphQL client to one easy to use package.
https://zino-hofmann.github.io/graphql-flutter
MIT License
3.25k stars 620 forks source link

feat(graphql): add raw to exceptions on QueryResult #1244

Closed maironLucasSlz closed 1 year ago

maironLucasSlz commented 2 years ago

A while ago, a coworker opened an issue reporting a need in our project about having access to the raw error to access some specific properties of it, such as the code for example. Recently, this issue was closed, but our need persists. With that, I decided to try to contribute to the package and add the parameter myself. First I researched the packages used and realized that the answer is obtained from the gql_exec package. In version 0.4.1 of this package there was a change in the Response class meeting an issue with similar needs to ours. This change makes the Response class now have an attribute also called response where the raw http of the response is. Using this new attribute, I propose a change in the assembly of the QueryResult class. The most viable solution I found is to include the list of errors received in the raw together in an attribute of the OperationException class that is added to the QueryResult in case of an error. For the needs of our project, it was a good solution, I hope it is approved and can contribute to the needs of other projects as well.

codecov[bot] commented 2 years ago

Codecov Report

Merging #1244 (24b6f43) into main (32f95c5) will decrease coverage by 4.13%. The diff coverage is 100.00%.

:exclamation: Current head 24b6f43 differs from pull request most recent head 1e9581e. Consider uploading reports for the commit 1e9581e to get more accurate results

@@            Coverage Diff             @@
##             main    #1244      +/-   ##
==========================================
- Coverage   63.47%   59.34%   -4.14%     
==========================================
  Files          41       41              
  Lines        1684     1685       +1     
==========================================
- Hits         1069     1000      -69     
- Misses        615      685      +70     
Impacted Files Coverage Δ
...es/graphql/lib/src/exceptions/exceptions_next.dart 16.07% <ø> (ø)
packages/graphql/lib/src/utilities/response.dart 53.33% <100.00%> (+7.17%) :arrow_up:
packages/graphql/lib/src/scheduler/scheduler.dart 4.16% <0.00%> (-95.84%) :arrow_down:
...ackages/graphql/lib/src/core/observable_query.dart 37.87% <0.00%> (-17.43%) :arrow_down:
packages/graphql/lib/src/core/query_options.dart 49.10% <0.00%> (-8.39%) :arrow_down:
...b/src/links/websocket_link/websocket_messages.dart 50.43% <0.00%> (-6.96%) :arrow_down:
packages/graphql/lib/src/core/query_manager.dart 76.19% <0.00%> (-1.52%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

maironLucasSlz commented 1 year ago

What's up, any update on this PR?

vincenzopalazzo commented 1 year ago

What's up, any update on this PR?

I give a shot this weekend, for the moment I'm happy if you remove the merge commit, thanks

maironLucasSlz commented 1 year ago

Hey. I removed the merge commit and deleted one duplicate commit. Do I need to do anything else?

vincenzopalazzo commented 1 year ago

I'm reviewing your PR, could you please rebase on the new main version?

I make a patch with https://github.com/zino-hofmann/graphql-flutter/pull/1261 that fix the CI

maironLucasSlz commented 1 year ago

I am not absolutely sure about that rebase I made, needed --force option on push. If has anything wrong, please tell me (as you can see, I am not too accustomed on contribute to open source projects)

vincenzopalazzo commented 1 year ago

I am not absolutely sure about that rebase I made, needed --force option on push. If has anything wrong, please tell me

Rebase always required the force push you did all steps correctly! Thanks! lets run the CI and if it is happy I merge it