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

add HEAD requests in RESTDataSource Class #27

Closed
devdudeio opened this issue Aug 7, 2019 · 9 comments · Fixed by #110
Closed

add HEAD requests in RESTDataSource Class #27

devdudeio opened this issue Aug 7, 2019 · 9 comments · Fixed by #110
Assignees

Comments

@devdudeio
Copy link

devdudeio commented Aug 7, 2019

Some endpoints of my REST API can be requested via a HEAD request but head is not implemented yet.

export class MyFancyAPI extends RESTDataSource {
    async countFancyDataset() {
        const response = await this.head(`countFancyDataset`); // myFancyDataset: 413212  (in Response Header and body will be empty)
        ...
    }
}
@devdudeio devdudeio changed the title add HEAD requests in datasource-rest add HEAD requests in RESTDataSource Class Aug 7, 2019
@abernix abernix self-assigned this Aug 26, 2019
@ghost
Copy link

ghost commented Oct 13, 2019

Is there any progress on this topic? I would love to use this enhancement :)

@shipraguptaa
Copy link

Is there any update on this? I also need to make HEAD method request using RESTDataSource but there is no support for that.

@shipraguptaa
Copy link

@abernix Any update?

@devdudeio
Copy link
Author

it’s not the best solution but if you need a fast solution just use fetch and do a HEAD request

@Phoenix1405
Copy link

@abernix Any update on this? We recently ran into this issue.

@abernix
Copy link
Member

abernix commented Nov 17, 2020

I don't have the precise answer as to the exact "why", but I could certainly believe that the omission of HEAD from the original implementation was because it wasn't fully realized that there was much in the way of data to be retrieved from such an endpoint (that doesn't return a body).

That said, data obviously comes in various forms and it's entirely possible to return certain valuable data as header values, so I think the need resonates with me too.

Thanks for opening the pull request, @Phoenix1405.

@devdudeio
Copy link
Author

I just passed the header into my returned data as a symbol

@kettanaito
Copy link

I believe HEAD requests can also compute data, just not return a body. That can be cached with RESTDataSource, so the addition of the HEAD method is quite practical.

@glasser glasser transferred this issue from apollographql/apollo-server Oct 11, 2022
@trevor-scheer
Copy link
Member

Ok, having familiarized myself with HEAD semantics I feel prepared to discuss this and propose a couple options.

  • HEAD bodies in responses must be ignored, meaning all of their useful data exists in the response headers. They ought to be treated as a GET request with no response body.
  • RESTDataSource in its current form doesn't expose the response object (just the parsed body) in its return value of fetch or the helper "http method" methods.

Based on this information, I'm going to assume people want access to the response headers when the make a HEAD request (else it's just an empty string). AFAICT, @Phoenix1405's PR mentioned above did not address this. I'm really not sure how useful this API addition can be otherwise, though.

I propose 2 possible options (with the option of 1, then later 2):

  1. Add a head method which breaks convention from the other convenience methods, i.e. it does not return a parsed body and instead returns either the whole response object or just its headers. This will result in some additional internal complexity around HEAD being special (i.e. the fetch method will have 2 different return types and have to look for HEAD, etc.). It also creates an inconsistency in the API (making it unique among the other convenience methods).
  2. In a breaking change to the API, start returning additional data in the convenience methods (either the whole response object alongside a parsed body or something similar, but at a bare minimum the headers as well).

I'd be interested in hearing input on these options from anyone on this thread. I'm leaning towards 2 myself since it offers a more robust and consistent API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants