Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Stop sending request response from the GraphQL proxy #312

Merged

Conversation

paulomarg
Copy link
Contributor

WHY are these changes introduced?

We noticed that the way the GraphQL proxy currently works is inconsistent with how the webhook process method works - while the webhook method returned the result of its operation, the proxy was triggering the actual response to be sent to the client, which was quite confusing.

WHAT is this pull request doing?

Changing the GraphQL proxy so it behaves consistently by returning the response of the queried request to the app, rather than taking over and responding to the request itself.

This allows apps to take the body of a proxied request and 1) respond however they prefer; 2) run any code they need to after sending the request.

Type of change

  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above
  • I have added/updated tests for this change

@paulomarg paulomarg requested a review from a team as a code owner March 2, 2022 21:02
@paulomarg paulomarg force-pushed the paulo/stop_returning_on_gql_proxy branch 3 times, most recently from b8a63ce to 22ca050 Compare March 2, 2022 21:05
Co-authored-by: Jason Kurian <jason.kurian@shopify.com>
@paulomarg paulomarg force-pushed the paulo/stop_returning_on_gql_proxy branch from 22ca050 to 08d881c Compare March 2, 2022 21:06
@paulomarg paulomarg merged commit fc33f0b into paulo/add_base_rest_resource Mar 2, 2022
@paulomarg paulomarg deleted the paulo/stop_returning_on_gql_proxy branch March 2, 2022 21:11
@tolgap
Copy link
Contributor

tolgap commented Mar 7, 2022

This seems to be a breaking change. But what actually breaks? The changelog doesn't mention what to do to if you upgrade.

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.

3 participants