This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Awaiting Schedule

These updates are awaiting their schedule. Click on a checkbox to get an update now.

• chore(deps): update all non-major dependencies (graphql, prettier)

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): update dependency eslint to v9
• chore(deps): update typescript-eslint monorepo to v7 (major) (@typescript-eslint/eslint-plugin, @typescript-eslint/parser)

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository

I'm using apollo server as an orchestration layer which connects and gets data from other backend servers.
I need to forward the set-cookie headers from backend server. Works well if there's one set-cookie header but if there are multiple there's a problem.

const { RESTDataSource } = require("apollo-datasource-rest");

class MyDataSource extends RESTDataSource {
  constructor() {
    super();
    this.baseURL = 'https://example.com';
  }

  didReceiveResponse(response, _request) {
    // here response.header is an Iterable and get method gives a string joining multiple header
    let cookies = response.headers.get("set-cookie");
    console.log(cookies);
    return super.didReceiveResponse(response, _request);
  }

  async resolverFunc(body) {
    // ...
  }
}

return type for response.headers.get is string | null but in this case it should be array.
if we console log response.headers we see that inside Symbol map the set-cookie header is actually an array but gets converted to string when trying to get the value using the get method.
Couldn't find a way to get the original array.

Package
[email protected]

Description
Apparently it can happen that an entry of the InMemoryLRUCache is an object, even though the entry is stringified before it is put in the cache. This causes this line to throw an error: line 56: const { policy: policyRaw, ttlOverride, body } = JSON.parse(entry);

Expected behavior
Check if the entry is an object so it does not have to be parsed and the code can continue.

Actual behavior
An error is thrown:

SyntaxError: Unexpected token o in JSON at position 1
[1]         at JSON.parse (<anonymous>)
[1]         at ContextHTTPCache.<anonymous> (C:\Users\***\node_modules\apollo-datasource-rest\src\HTTPCache.ts:48:9)
[1]         at Generator.next (<anonymous>)
[1]         at fulfilled (C:\Users\***\node_modules\apollo-datasource-rest\dist\HTTPCache.js:4:58)
[1]         at processTicksAndRejections (internal/process/task_queues.js:97:5)

apollographql/apollo-server#4353

apollo-datasource-rest has a HTTPCache that probably works fine, but it is never invoked because RESTDataSource caches the promise itself, the first time it is called (for GET requests).

This seems to defeat the purpose of having HTTPCache altogether, but looking at the Git history, a good deal of work was put into this memoization, so maybe there is a reason for it?

https://github.com/apollographql/apollo-server/blob/f25784d51130bbb9014b727e2f231e5a737f4a50/packages/apollo-datasource-rest/src/RESTDataSource.ts#L270

We have moved from request-promise to apollo-datasource-rest module in our project. The request-promise module was doing the job for us by omitting the undefined variables inside a query param object.

For example if we had something like:

{ param1: 'value', param2: undefined }

the resulting URL was:

https://domain.com/test-url?param1=value

But in case of apollo-datasource-rest we are getting:

https://domain.com/test-url?param1=value&param2=undefined

Which is breaking the existing tests in our project. We can omit the property at our side but it would be great if it would omit the undefined query params automagically ;)

I have a suggestion to add processing of 422 status response in errorFromResponse and throw UserInputError in case of it.

  protected async errorFromResponse(response: Response) {
    const message = `${response.status}: ${response.statusText}`;

    let error: ApolloError;
    if (response.status === 401) {
      error = new AuthenticationError(message);
    } else if (response.status === 403) {
      error = new ForbiddenError(message);
    // add next lines:
    } else if (response.status === 422) {
      error = new UserInputError(this.getValidationErrorMessage(), this.parseValidationErrors(response));
    } else {
      error = new ApolloError(message);
    }

    const body = await this.parseBody(response);

    Object.assign(error.extensions, {
      response: {
        url: response.url,
        status: response.status,
        statusText: response.statusText,
        body,
      },
    });

    return error;
  }

I am attempting to hit a REST endpoint with multiple URL parameter values concatenated via either "+" or "," - specifically a Wordpress server. But the Apollo Get datasource call removes both of them from the URL, with what I would assume is an attempt at making the URL safe. Is there an option to prevent it from converting + and , in a Get request.

My request:

let args = { per_page: 50,
  tags: '2551+2552',
  orderby: 'date',
  order: 'desc' }
result = await this.get("posts", args);

The URL Generated:

http://my-wordpress-site/wp-json/wp/v2/posts?per_page=50&tags=2551%2B2552&orderby=date&order=desc

It should be: http://my-wordpress-site/wp-json/wp/v2/posts?per_page=50&tags=2551+2552&orderby=date&order=desc

Version being used is 0.9.0

I'm trying to use didEncounterError(.. to get the JSON from the back-end API in case of any error but looks like the message doesn't get added on to the error/extensions obj

How can I get a handle on the JSON being returned by the back-end API when there is a 4xx or 500 error?

Version
APOLLO RESTDATASOUCE = apollo-datasource-rest": "^0.9.3",
APOLLO SERVER = "apollo-server-lambda": "^2.16.1",
TYPESCRIPT = ""typescript": "^4.0.2""

I trying follow the documentation steps to set a dynamic url based on my ENV (setted in context)
But its show warning message 'baseURL' is defined as a property in class 'RESTDataSource<Context>', but is overridden here in 'TransactionsAPI' as an accessor.

Snippet:

export default class TransactionsAPI extends RESTDataSource<Context> {
  constructor() {
    super();
  }

  get baseURL() {
    if (this.context.env === "development") {
      return "https://dev.api_example.com.br";
    } else if (this.context.env === "production") {
      return "https://prod.api_example.com.br";
    }
  }
  async myFunction() {
   return this.post(`/patch`)
  }
}

Im following this steps
http://apollographql.com/docs/apollo-server/data/data-sources/#resolving-urls-dynamically

Thank you!

• Package name and version

"apollo-datasource-rest": "^0.13.0",
    "apollo-server": "^2.24.1",
    "graphql": "^15.5.0"

• Expected behavior

  • Expected it to not cache the api response when the response has header cache-control: no-cache
  • Expected it to not cache the api response when explicitly configured to do so. Example
    this.get(`https://randomuser.me/api/`, undefined, { cache: "no-cache", });
this.get(`https://randomuser.me/api/`, undefined, { cache: "no-store", });
this.get(`https://randomuser.me/api/`, undefined, { cache: "no-cache", cacheOptions: { ttl: 0 }, });

• Actual behavior
  - The api response is cached in either of the cases.
  - I have demonstrated the caching behavior with the time it takes to fulfill the subsequent request in the exaple repo given below
  - The actual time may depend on the connection speed and location. But the overall pattern remains the same
  - Please also refer to the screenshots

• Repo for reproduction
  Refer to the instructions given in the readme https://github.com/shreyas1496/apollo-cache-issue

Expected logs in console

Actual logs in console

I use apollo-datasource-rest to cache responses from remote REST-APIs. Responses that have a 404 status get cached and thus read from cached on subsequent calls to that same endpoint. This is fine, until the error for the request gets created. The first request - the one that actually hit the remote and not the cache - creates an error like this:

      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "response": {
          "url": "http://localhost:4000/someResource",
          "status": 404,
          "statusText": "Not Found",
          "body": "Not found"
        },
        "exception": {
          "stacktrace": [
            "Error: 404: Not Found",
            "    at ExampleDataSource.<anonymous> (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:84:25)",
            "    at Generator.next (<anonymous>)",
            "    at /sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:8:71",
            "    at new Promise (<anonymous>)",
            "    at __awaiter (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:4:12)",
            "    at ExampleDataSource.errorFromResponse (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:74:16)",
            "    at ExampleDataSource.<anonymous> (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:52:34)",
            "    at Generator.next (<anonymous>)",
            "    at /sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:8:71",
            "    at new Promise (<anonymous>)"
          ]
        }
      }

The important bit is the url-field telling us which exact request failed. The actual issue lies in the second request - the one that hits the cache -, which creates the following error:

      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "response": {
          "status": 404,
          "statusText": "Not Found",
          "body": "Not found"
        },
        "exception": {
          "stacktrace": [
            "Error: 404: Not Found",
            "    at ExampleDataSource.<anonymous> (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:84:25)",
            "    at Generator.next (<anonymous>)",
            "    at /sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:8:71",
            "    at new Promise (<anonymous>)",
            "    at __awaiter (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:4:12)",
            "    at ExampleDataSource.errorFromResponse (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:74:16)",
            "    at ExampleDataSource.<anonymous> (/sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:52:34)",
            "    at Generator.next (<anonymous>)",
            "    at /sandbox/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:8:71",
            "    at new Promise (<anonymous>)"
          ]
        }
      }

As you can see, the url-field is missing. Interestingly enough, this only happens if i set a ttl in the RequestInit object I pass to the datasource. I prepared a small reproduction for this issue: https://codesandbox.io/s/little-microservice-hm0sv

I analyzed the source and believe the line causing the issue is this one: https://github.com/apollographql/apollo-server/blob/c177acd7959aaf516f8ac814f1a6bb2e6d3ed2e6/packages/apollo-datasource-rest/src/HTTPCache.ts#L60
Before spending too much time looking for possible solutions, I wanted to make sure that this is in fact an issue and not something you actually intended because of some piece of information that I'm currently missing.

In package apollo-datasource-rest, the datasource is incorrectly storing a 302 redirect with the caching headers supplied by the final resource. Then when the request is checked, the original URL is assumed to be fresh and it does not check the original redirect again. This goes against the HTTP spec for 302 redirects (and caused a problem for me in my app)

I've created an integration spec that demonstrates the problem in my fork: https://github.com/gburgett/apollo-server/blob/rest-caching/packages/apollo-datasource-rest/src/__tests__/integration.test.ts

To reproduce:

• Set up a REST API with a 302 redirect to some static resource with cache-control: public, max-age=31536000, immutable
• Issue a GraphQL query that performs a GET request to this 302 redirect path
• Change the 302 redirect to point to another resource
• Issue the same GraphQL query again

At this point, the expected behavior is that the new resource is returned. The actual behavior is that the old resource is served from the HTTP cache.

Our current workaround is to set the TTL to 0 in the caching options for this data source.

Right now, because of this line, revalidation using HTTP Cache Headers only works with ETags.

Shouldn't Last-Modified/If-Last-Modified also be allowed in there ?

I need to fetch raw image data from a service and then return a base64 encoded version of that data to the front end. After testing, I discovered that the stock node-fetch implementation is mangling the data without the appropriate response type. There doesn't appear to currently be a way with RESTDatasource to resolve the different types that fetch supports (arraybuffer, blob, text, json, etc).

Is this something that can be added in?

This page of the docs for rest data sources isn't working anymore:

https://www.apollographql.com/docs/apollo-server/serving/data-sources/#rest-data-source

I see that the page still exists in the docs which makes me think this is an error:

https://github.com/apollographql/apollo-server/blob/master/docs/source/data/data-sources.md

Was it moved somewhere else?

path : packages/apollo-datasource-rest/src/HTTPCache.ts line 111
issue : missing await
current code :
if (typeof cacheOptions === 'function') {
cacheOptions = cacheOptions(response, request);
}

change required :
if (typeof cacheOptions === 'function') {
cacheOptions = await cacheOptions(response, request);
}

actual behavior : when we are sending cacheOptions as function and trying to read ttl value from response object (response.json()) , it is returning promise. and caching is not working with ttl.

expected behavior : when we override cacheOptions function and read ttl value from response object , it should wait for promise to resolve. so caching working properly with ttl.

NOTE : With this minor change it will work both way whether cacheOption function is returning promise or not.

This issue is in latest version and i think in all previous version.

reproduce :

this.post(url, body, {
headers ,
cacheOptions: async (response) => {
const res = await response.clone().json()
return { ttl: parseInt(res.expiryTimeInSeconds) }
}
});

apollo-datasource-rest has a mechanism to extend the TTL of a specific cache entry if the response can be revalidated.

Right now the method checks only for the presence of the ETag header, but the library used to extract the revalidation headers has a more complex handling: https://github.com/kornelski/http-cache-semantics/blob/master/index.js#L517

It may be useful to extend the canBeRevalidated method to include at least a check for last-modified header presence.

How can I read the header via apollo-datasource-rest from a response? I have a question posted on stackoverflow but no luck so far

https://stackoverflow.com/questions/60764956/read-header-from-response-with-apollo-datasource-rest

Package version
"apollo-datasource-rest": "^0.6.11"

Expected behavior
When a GET request is made using RESTDataSource and a ttl value is provided, the response is cached for the given time.

const response = await this.get('some-path', {}, { cacheOptions: { ttl: 600 } });

Then a second request is made while the request is still cached, but the reload option is given:

const response = await this.get('some-path', {}, { cache: 'reload', cacheOptions: { ttl: 600 } });

I am expecting this will fetch the response from the datasource and then save the response to the cache with a ttl of 600.

Actual behaviour
The response from the second request is returned from the cache (and the cache isn't updated). I believe that this if statement here should check whether the request needs to be remade regardless of the ttlOverride from the first cached request.

Please let me know if this behavior is deliberate.

Thanks!

I am using apollo server with RESTDataSource to call the Rest APIs. When I did the load testing it is having lot of connection waits. I believe connection is not getting closed.

I have a data source like this:

class MyApi extends RESTDataSource {
  constructor() {
    super();
    this.baseURL = 'https://my-api.example.com/';
  }

  async getUser(query) {
    return this.get('/user', query);
  }
}

This is my resolver:

const userResolver = (_, __, {dataSource: {myApi}}) => {
  const query = {
    userId: "1",
    optionalParam: undefined,
  };
  return myApi.getUser(query);
}

This leads to this HTTP request:

GET /myApi/user?userId=1&optionalParam=undefined

while I think it would be more logical if this was fetched:

GET /myApi/user?userId=1

I think these lines implement the behavior:

https://github.com/apollographql/apollo-server/blob/d3882350fb768bde2d599343c257dc71400cb360/packages/apollo-datasource-rest/src/RESTDataSource.ts#L226-L229

Caching in progress requests issue
There appears to be a race condition between two simultaneous calls to the same url endpoint from separate queries. I outlined the situation below.

Setup:

• Two resolvers that fetch data from the same url endpoint from a single datasource simultaneously.
• The datasource extends RESTDataSource and uses the get() method to fetch the data and cache the url endpoint and resulting promise.

Intended outcome:

• There should be a single request to the http server

Actual Outcome:

• We see two requests to the http server
• Since the apollo cache gets populated only when the promise is resolved, the second call can’t use the cache. Both requests fetch the data. This is especially problematic with a slow url endpoint. It results in double the time.

The issue reproduced in the following project: https://github.com/kowther/CashingInProgressEventIssue

This bug report should include:

• A short, but descriptive title. The title doesn't need "Apollo" in it.
• The package name and version of Apollo showing the problem.
• If applicable, the last version of Apollo where the problem did not occur.
• The expected behavior.
• The actual behavior.
• A simple, runnable reproduction!
  Please make a GitHub repository that anyone can clone and run and see the
  problem. Other great ways to demonstrate a reproduction are using our
  CodeSandbox template (https://codesandbox.io/s/apollo-server), or re-mixing
  our Glitch template (https://glitch.com/~apollo-launchpad).
  -->

First off, you DO NOT explain how one can obtain info you demand above.
Hence I only add info I know how to obtain.

server/package.json

  "dependencies": {
    "apollo-datasource": "^0.1.3",
    "apollo-datasource-rest": "^0.1.5",
    "apollo-server": "2.6.1",
    "apollo-server-testing": "2.6.1",
    "graphql": "^14.2.1",
    "isemail": "^3.1.3",
    "nodemon": "^1.18.4",
    "sequelize": "^4.39.0",
    "sqlite3": "^4.0.3"
  },

apollo-datasource-rest throws error in GET requersts

The expected behavior.
const {RESTDataSource} = require('apollo-datasource-rest'); can make REST request without errors.

The actual behavior.

(node:30845) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'fetch' of undefined
    at LaunchAPI.<anonymous> (/Users/admin//apollo/fullstack-tutorial/start/server/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:148:63)
    at Generator.next (<anonymous>)
    at /Users/admin//apollo/fullstack-tutorial/start/server/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/admin//apollo/fullstack-tutorial/start/server/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:3:12)
    at trace (/Users/admin//apollo/fullstack-tutorial/start/server/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:143:78)
    at LaunchAPI.<anonymous> (/Users/admin//apollo/fullstack-tutorial/start/server/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:186:24)
    at Generator.next (<anonymous>)
    at /Users/admin//apollo/fullstack-tutorial/start/server/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:7:71
    at new Promise (<anonymous>)
(node:30845) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:30845) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

A simple, runnable reproduction!

1. Follow your Tutorial
2. Follow this step Hook up your data sources up to Connect a database section (don't implement it)
3. Open index.js file and add the following lines:

const api = new LaunchAPI();
const launches = api.getAllLaunches(); // <== THIS THROWS

Why it doesn't work?
What is property 'fetch' of undefined?

Related issue
apollographql/apollo-server#3429

Right now the didReceiveResponse and errorFromResponse pair works fairly well with non-2xx statuses. Unfortunately there exists REST services (legacy) which return 200 OK with error object instead as body. If trying to check this in didReceiveResponse and then throwing error through errorFromResponse causes problem with the response.json() parsing, as you cannot call that one twice.

Either the parseBody should know better (processing time memoize) or errorFromResponse should accept body passed and use that instead of calling parseBody. This would allow passing already parsed body to be passed to super.didReceiveResponse.

Right now my solution is just to copy-paste (partially) code for of ApolloError object create from errorFromResponse to the custom didReceiveResponse.

Package: apollo-datasource-rest

It would be useful to emit metrics around caching. More specifically, some boolean value that indicates when a request results from the cache vs HTTP. In this way, I would be able to emit metrics for my application and adjust TTLs based on those statistics https://github.com/apollographql/apollo-server/blob/master/packages/apollo-datasource-rest/src/HTTPCache.ts#L116-L146

Thank you

Description

We're using apollo-datasource-rest alongside apollo-server to manage external REST requests.

We noticed that using any TTL value when doing a request leads to several memory leaks.

Here is an allocation timeline of the server while using {cacheOptions: {ttl: 1}}

Timeline using {cacheOptions: {ttl: -1}}

We used 1 for this example but any other value creates same leaks in our observations.

The memory usage is quite stable in case of TTL -1:

But still growing in case of TTL 1:

Version Info

apollo-server: 2.13.0
apollo-datasource-rest: 0.9.0

Expected Behavior

When TTL = 1 I'm expecting cached requests to be removed from memory after 1 sec but the InMemoryLRUCache store is still growing in my case.

Steps

yarn i
yarn run build
node --inspect dist/index.js
# open chrome://inspect in chrome
# send queries

Here is the server we use:

# index.js
const server = new ApolloServer({
    schema: MySchema,
    dataSources: {
       myAPI: new MyAPI({baseURL: ...})
    },
    context: ...,
    cacheControl: {
      defaultMaxAge: 86400,
    },
  });

Datasource:

export default class MyAPI extends RESTDataSource {
  constructor(options) {
    super();
    this.baseURL = options.baseURL;
  }

  async getItem(id) {
    const apiResult = await this.get(
      `/MyEndpointPath/${id}`,
      {
        aGetParam: 'getParamValue',
      },
      { cacheOptions: { ttl: 1 } },
    );
  }
  return {
     name: apiResult.title
  };
}

Resolver

export default {
  Query: {
       getItem(_, args, context): object {
              return context.dataSources.myAPI.getItem(args.id);
         },
   }
}

Schema

extend type Query {
  getItem(id: String): Item
}

type Item @cacheControl(maxAge: 86400) {
    name: String
}

Is it expected behavior or did we miss a part of the configuration ?

Hi,
I have question about RESTDataSource docs:
https://www.apollographql.com/docs/apollo-server/features/data-sources.html#HTTP-Methods

What have you passed as a movie in post/put/patch requests? Because it seems that you can't pass object there. You should JSON.strigify it first. Otherwise you will get "cannot convert object to primitive" from node-fetch. Am I right?

Here is my data source:

export default class DataSource extends BaseDataSource {
    uri = '/travelSeasons';

    async getTravelSeasons() {
        return this.get(this.uri);
    }

    async addTravelSeason(travelSeason: TravelSeason) {
        return this.post<TravelSeason>(this.uri, travelSeason);
    }

    async updateTravelSeason(travelSeason: TravelSeason) {
        return this.patch<TravelSeason>(this.uri + `/${travelSeason.id}`, travelSeason);
    }
}

And when I comment lines below:

export default class BaseDataSource extends RESTDataSource {
    baseURL = config.API_URL;

    // protected post<TResult = any>(path: string, body?: Body, init?): Promise<TResult> {
    //     return super.post(path, JSON.stringify(body), {
    //         headers: {
    //             "Content-Type": "application/json"
    //         },
    //         ...init
    //     });
    // }

    ...
}

I will get this:

{
  "data": {
    "addTravelSeason": null
  },
  "errors": [
    {
      "message": "Cannot convert object to primitive value",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "addTravelSeason"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "stacktrace": [
            "TypeError: Cannot convert object to primitive value",
            "    at String (<anonymous>)",
            "    at Request.Body (/usr/src/app/node_modules/node-fetch/lib/index.js:172:10)",
            "    at new Request (/usr/src/app/node_modules/node-fetch/lib/index.js:1169:8)",
            "    at DataSource.<anonymous> (/usr/src/app/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:140:29)",
            "    at Generator.next (<anonymous>)",
            "    at fulfilled (/usr/src/app/node_modules/apollo-datasource-rest/dist/RESTDataSource.js:4:58)",
            "    at <anonymous>",
            "    at process._tickCallback (internal/process/next_tick.js:188:7)"
          ]
        }
      }
    }
  ]
}

My node version is: 8.11.3, all libraries are up to date.

Btw what about "Content-Type": "application/json" header? Is there a good way to add it to my datasource by default (in base class or sth - I guess some kind of adapter) ? Is my solution ok?

We closed apollographql/apollo-server#5015 to move it to this repository. We should look to see if the change is still applicable.

I would like to use cache keys with request body for POST requests.

  protected cacheKeyFor(request: Request): string {
    return request.text(); // This type is Promise<string>
  }

Current cacheKeyFor interface in RESTDataSource provides a request object but it seems it doesn't contains simple body text.
Please help.

package

apollo-datasource-rest: 0.9.3

Expected behaviour

The content-type header, if not present, is added when a valid JSON is provided in the body.

Actual behaviour

The content-type header, if not present, is added only if the body contains an object, array or date.

affected code

A simple example (please pay no attention to the validity of the queries)

The point is to debug the provided affected code when the query
query{ hello bye }
is executed.

The hello query passes a string in the body and the bye one passes an object. They both use the datasource's put.

First request:

Second Request:

The RESTDataSource class automatically explicitly declares a default type of any https://github.com/apollographql/apollo-server/blob/master/packages/apollo-datasource-rest/src/RESTDataSource.ts#L47 for the async functions get, post, etc

We should force the user to declare the type. The library should not assume any.

As a workaround, I've been doing this, but it's very inconvenient because the code compiles and the type is simply inferred as unknown, which isn't immediately obvious to the consumer:

export class BaseDataSource extends RESTDataSource {
  constructor() {
    super();
  }

  protected async get<TResult>(...args: Parameters<RESTDataSource['get']>) {
    return super.get<TResult>(...args);
  }

  protected async post<TResult>(...args: Parameters<RESTDataSource['post']>) {
    return super.post<TResult>(...args);
  }

  protected async patch<TResult>(...args: Parameters<RESTDataSource['patch']>) {
    return super.patch<TResult>(...args);
  }

  protected async put<TResult>(...args: Parameters<RESTDataSource['put']>) {
    return super.put<TResult>(...args);
  }

  protected async delete<TResult>(...args: Parameters<RESTDataSource['delete']>) {
    return super.delete<TResult>(...args);
  }
}

My app downloads a lot information from multiple external webpages. The problem is that one of them they doesn't have an API. So I making a simple GET request to their webpage but that request return a lot of HTML that I parse and extract like 10 characters. The problem is that cache stores everything and it's so much unnecessary information.

I was thinking if there was possible to have something like

const response = await this.get(
  '/some/url',
  { some: 'query' },
  {
    cacheOptions: {
      ttl: 60 * 60 * 24,
    },
    // Want something like this to be executed once for each request but not when using cache.
    transform: (body) => { 
      const $ = cheerio.load(body);
      return $('.some-selector').text(); // Only cache this.
    }
  },
);
response // Should equal $('.some-selector').text() result

It could also be good transform big json objects into smaller object or fix bad data structure that doesn't fit your application.

I don't want todo something like this because it's inefficient:

const response = await this.get(
  '/some/url',
  { some: 'query' },
  {
    cacheOptions: {
      ttl: 60 * 60 * 24,
    },
  },
);
const $ = cheerio.load(response);
const result = $('.some-selector').text();

Hey folks 👋 We have an existing subclass of RESTDataSource that logs a variety of metrics for each call to fetch. We're trying to instrument our data sources to better understand how caching/memoization is used in production. However, RESTDataSource doesn't make it easy to figure out this information; the best we could do was manually querying the cache and memoizedResults to try to infer what's happening. However, in the end, we ended up forking RESTDataSource/HTTPCache to make cache status information first-class data in the return values from get/post/etc. We defined a new type, FetchResult that wraps the original response with cache metadata:

export interface FetchResult<TResult> {
  context: {
    cacheHit: boolean;
    memoized: boolean;
  };
  response: Promise<TResult>;
}

We then updated the get/post/etc. to return a FetchResult:

  protected async get<TResult = any>(
    path: string,
    params?: URLSearchParamsInit,
    init?: RequestInit
  ): Promise<FetchResult<TResult>> {
    return this.fetch<TResult>(
      Object.assign({ method: 'GET', path, params }, init)
    );
  }

Finally, we changed RESTDataSource#fetch and HTTPCache#fetch to return objects with that same context property. With this, we could update our subclass of RESTDataSource to automatically report whether particular requests were served by the cache or were memoized.

Here's our implementation in a Gist: https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5. The important bits:

• definition of FetchResult
• updated get/etc.
• returning cacheHit: false from HTTPCache#fetch
• returning cacheHit: true from HTTPCache#fetch
• returning cacheHit for revalidated cache data
• consuming cache status from HTTPCache
• returning memoized: true for memoized requests

While this works, it's less than ideal to have to fork RESTDataSource and HTTPCache, since that introduces additional maintenance burden on our team. Ideally, this could be provided by the apollo-datasource-rest package itself. Does Apollo have any interest in adding this functionality? It doesn't necessarily need to use the same FetchResult interface we invented, but we'd appreciate anything that would give us more insight into how the cache is used.

We closed apollographql/apollo-server#3806 to move it to this repository. We should look to see if the change is still applicable.

the apollo-datasource package doesn't have any reference documentation on the docs site. The full-stack tutorial give a few "helpful implementation" tips for building your own, but it's far from an API reference, or even detailed enough to say what interface should be expected for a custom datasource.

I'm just as capable as reading source code as the next guy, but with all the "v3 is coming" and package distribution renaming and everything... having absolutely no api reference left me confused as to whether that package was being deprecated in favor of something else.

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)
        ...
    }
}

We closed apollographql/apollo-server#4176 to move it to this repository. We should look to see if the change is still applicable.

// CODE BELOW DOESN'T WORK
await this.delete(`contacts/${contactId}`, undefined, { headers: this.context.req.headers });
await this.fetch({ method: 'DELETE', path: `contacts/${contactId}`, headers: this.context.req.headers });

// CODE BELOW WORKS
await this.delete(`contacts/${contactId}`);

Why I cannot set a custom header to do a delete operation? For post and patch actions works fine, but no for delete. Why is that so?

https://github.com/apollographql/apollo-server/blob/master/packages/apollo-datasource-rest/src/RESTDataSource.ts#L180
this method doesn't have body param to pas to request. could you add it? there's no reason to not support it because delete request supports it:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/DELETE

I know that the use of it is not recommended, but it's it's not prohibited either. and we're dealing with API that is using this kind of thing.

also, fetch method is private. so if any of the provided methods doesn't suit our needs, we're blocked from using this class.
https://github.com/apollographql/apollo-server/blob/master/packages/apollo-datasource-rest/src/RESTDataSource.ts#L190
also other methods (like post, put or patch) doesn't accept params property. so I don't understand why you're limiting other methods only to body params.

Apollo version: 2

RESTDataSource.resolveUrl doesn't work correctly for paths with a colon, e.g.

this.get('/foo:')

this throws

TypeError: Only absolute URLs are supported

The source code strips out the leading slash, which leads to the call new URL('foo:', 'http://example.com'), where foo: looks like an absolute URL without a host.

https://github.com/apollographql/apollo-server/blob/cd19cfeff260c4b948fb56f0b3b210a43858cc8c/packages/apollo-datasource-rest/src/RESTDataSource.ts#L74-L88

I would like to be able to tell what resolved an API call. This can currently be resolved by either the in-memory cache, and external cache, or the actual source. This is needed for a billing log I am needing to write and I accumulate the data as the request is being processed (in my datasource code). This could be a feature that is 'turned on' somehow, so that users that do not need the feature will not enable it.

This is a bug report.

• Package: apollo-datasource-rest
• Version: latest

Fetching resources that are url-like does not work as expected.

Unit test to reproduce:

it('fails with urn-like resource', async () => {
      const dataSource = new (class extends RESTDataSource {
        baseURL = 'https://api.example.com';

        getFoo() {
          return this.get('/urn:foo:1');
        }
      })();

      dataSource.httpCache = httpCache;

      fetch.mockJSONResponseOnce();

      await dataSource.getFoo();

      expect(fetch.mock.calls.length).toEqual(1);
      expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/urn:foo:1');
    });

The issue is caused by a combination of URL behavior and normalization logic in
https://github.com/apollographql/apollo-server/blob/master/packages/apollo-datasource-rest/src/RESTDataSource.ts#L74-L88

A better approach would be to remove an ending / from baseURL and add a leading / to path. E.g.

protected resolveURL(request: RequestOptions): ValueOrPromise<URL> {
    const normalizedPath = request.path.startsWith('/')
      ? request.path
      : request.path.concat('/');
    
    const baseURL = this.baseURL;
    if (baseURL) {
      const normalizedBaseURL = baseURL.endsWith('/')
        ? baseURL.substring(0, baseURL.length - 1)
        : baseURL;
      return new URL(normalizedPath, normalizedBaseURL);
    } else {
      return new URL(normalizedPath);
    }
}

however it breaks a few existing tests.

What:
Every HTTP request writes in the console an entry. The only way to turn it off is by changing the process.env.NODE_ENV

Issue:
We should be able to turn it off in development.

How:
RESTDataSource can have an option to enable or disable the tracing.

Where:
Exactly on that line: https://github.com/apollographql/apollo-server/blob/master/packages/apollo-datasource-rest/src/RESTDataSource.ts#L271

At the moment, it checks for "development" but it still should let the developer to turn it off.

apollo-datasource-rest fetch types are out of sync with the used implementation. For example, the signal property is missing.

https://github.com/node-fetch/node-fetch/tree/2.x#request-cancellation-with-abortsignal

We closed apollographql/apollo-server#4306 to move it to this repository. It seems like a reasonable change that we should consider implementing.

cc @harleynuss

We closed apollographql/apollo-server#4660 to move it to this repository. We should look to see if the change is still applicable.

From my understanding of GraphQL data that is not available as part of a request should be returned as null, therefore allowing other parts of the query to return for the client to render as best.

If this is the case and your rest API returns 404 status codes however you'll need to write something like the following to capture the errors and return null;

      try {
        return await dataSources.listingApi.getListing(args.reference);
      } catch (err) {
        
        if(err.extensions.response.status === 404) {
          return null;
        }
        throw err;
      }

This works but isn't particularly elegant for what seems to be a common use case. Have I misunderstood how to deal with 404 errors in GraphQL or is this part of the rest datasource that could have better handling around it?

I'd be interested to know how other people are dealing with this in a more elegant manor or at least encapsulating the null return inside the class which extends RESTDataSource. From skimming the code this didn't look easy to handle without rewriting some of the protected methods.

RESTDataSource caches each GET request, and clears the cache for other HTTP verbs. Most of the time this is exactly what users need. However there are cases where users need to cache POST requests as if they were query-style requests, for example, if they are POSTing a graphql query, or if the resource they are requesting accepts POST bodies as configuration for some computation. In these cases, users would like to configure the caching behaviour on a per-request basis.

API Proposal

class MyDataSource extends RESTDataSource<MyContext> {
  /** @override */
  protected checkRequestCache<T = any>(
    request: Request,
    cacheKey: string,
    performRequest: () => Promise<T>,
  ): Promise<T> {
    if (request.url.includes('/my-posty-query-endpoint')) {
      let promise = this.memoizedResults.get(cacheKey);
      if (promise) return promise;
      promise = performRequest();
      this.memoizedResults.set(cacheKey, promise);
      return promise;
    } else {
      return super.checkRequestCache(request, cacheKey, performRequest);
    }
  }
}

Absent this feature, the user will have to override the entire fetch method, when she really only wants to modify the last 1/5 of its code. Moreover, fetch is private and can't be overridden, so users with this case seemingly have no recourse.

Alternate API Proposal

class RESTDataSource {
  private async fetch<TResult>(
    init: RequestInit & {
      path: string;
      params?: URLSearchParamsInit;
    },
  ): Promise<TResult> {
    // ... yadda yadda ...
  
    const performRequest = async () => { /* ... */ };
  
    if (this.shouldCacheRequest(request)) {
      return this.cacheRequest(cacheKey, performRequest)
    } else {
      this.memoizedResults.delete(cacheKey);
      return performRequest();
    }
  }
  
  /** Override to configure caching behaviour per request */
  protected shouldCacheRequest(request: Request): boolean {
    return request.method === 'GET';
  }
  
  private cacheRequest<T = any>(cacheKey: string, performRequest: () => Promise<T>): Promise<T> {
    let promise = this.memoizedResults.get(cacheKey);
    if (promise) return promise;
  
    promise = performRequest();
    this.memoizedResults.set(cacheKey, promise);
    return promise;
  }
}

Usage:

protected shouldCacheRequest(request: Request): boolean {
  const { pathname } = new URL(request.url);
  switch (pathname) {
    case '/query-post':
    case '/definitely-cache-this':
      return true;
    default:
      return request.method === 'GET';
  }
}

We closed apollographql/apollo-server#4816 to move it to this repository. We should look to see if the change is still applicable.

I'm currently refactoring some REST API to use RESTDataSource as the fetch implementation. Current implementation uses similar custom rest/fetch implementation, which is fairly okay but does have it's caveats and does not provide the caching features.

However it does provide way to pass custom context on request/response pair. This is used for (in RESTDataSource terms) the parseBody to do more detailed json data denormalization. I was hoping I could implement this just extending the RESTDataSource, but seems it's not that easy as the request/response pair is being detached from each other. The didReceiveResponse does link up a bit, but as how the node-fetch Request and Response are internally initialized, any custom additions to the init: RequestInit parameter are not passed through just with Request and Response objects.

Right now I see only three options for this.

1. FEATURE REQUEST: RESTDataSource should have way to pass "request context" which is passed through the pipe down to parseBody. This would work then when extending from RESTDataSource
2. Hackly solution I'm trying is to use local Map to store the data and link by "request-id" which is passed through the request headers and then copied in didReceiveResponse to response headers which can be accessed in parseBody.
3. Don't try to extend RESTDataSource, just create wrapper class and use RESTDataSource as it-is for any fetches, post process the responses result data in the wrapper

Because lack of direct support for option 1, I'm a bit leaning towards the option 3, although the RESTDataSource code seems to be done extending in mind. So probably have to extend RESTDataSource for some customzation and then wrap it..

Other suggestions?