File uploads breaking when upgrading to Apollo Server 4

Help
1

Hi,

I use Apollo Server with Fastify and graphql-upload and am currently trying to upgrade to Apollo Server 4, Fastify 4 (with @as-integrations/fastify) and graphql-upload 16.

When upgrading from version 2 to 3 of Apollo and Fastify, using graphql-upload 12, I followed this example for uploads which worked right away: File uploads - Apollo GraphQL Docs.

Details: in the React client we have an input of type “file”. The added files are sent to Apollo Server as a binary. As described in the linked document above, we have a preValidation hook for multipart requests, which applies to file uploads. In there we call the processRequest method of graphql-upload because it doesn’t provide middleware for Fastify. The result of that goes to the resolver. Inside the resolver we call createReadStream on the file and pass the result to the default export of the raw-body npm package, which gets the entire buffer of a stream either as a Buffer or a string. We then pass this buffer to the RESTDataSource. The upload method inside the RESTDataSource uses FormData (currently form-date from npm but also tried native FormData) and adds the buffer to the body for the request we send via the RESTDataSource’s post method.

Now, when trying to upgrade to version 4, this doesn’t work anymore (it’s not the CSRF issue). But without making any other changes apart from the upgrades, the data source server suddenly complained about an empty (or non-existent) Content-Type header. I tried adding it manually incl. a generated boundary. But then the server started complaining about a missing parameter. Since form-data didn’t change, I wouldn’t know why the body of the request should be different unless something different is happening inside RESTDataSource’s post method.

Reverting to an older version of graphql-upload and raw-body also didn’t change anything there, so it seems like it must be caused by the Apollo upgrade (potentially a change to RESTDataSource). But so far I couldn’t find any info on what I would have to do differently.

I also briefly tried using fastify-multipart. But then graphql-upload’s processRequest started throwing an error. Or rather, the underlying busboy package did: Error: Unexpected end of form at Multipart._final (node_modules/busboy/lib/types/multipart.js:588:17). Wasn’t able to fix that either unfortunately.

2

Do you have a reproduction you can share? There are a lot of moving parts here so being able to reproduce it on my end will be very helpful.

4

Thanks for your reply, @trevor.scheer. Unfortunately my answer to your comment was removed by the spam filter for some reason and hasn’t reappeared yet… :smiling_face_with_tear:

7

OK, another try at replying (seems enough to edit a post for it to be removed by the spam filter)… Unfortunately the repos are from work and thus private. But as mentioned in my original post, we followed the upload guide for Apollo 3 and initially just upgraded the dependencies. The addContentTypeParser for multipart and addHook for preValidation still look the same. Inside the hook we call graphql-upload’s processRequest and assign the result to the request body.

In the resolver we then do this:

            const adsFileUpload: IFileUpload = await args.adsFile.file;
            const adsFileReadStream = adsFileUpload.createReadStream();
            const adsFileBuffer = await rawBody(adsFileReadStream);

rawBody is from the raw-body nom package.

The adsFileBuffer is passed on to the upload method inside the RESTDataSource, where we do this:

        const body = new FormData();
        body.append('categoryId', String(categoryId));
        body.append(
            'adsFile',
            new Blob([adsFile.buffer], { type: adsFile.contentType }),
            adsFile.filename
        );

This is using the native FormData (thus the conversion to a Blob) whereas before we used the form-data npm package which is outdated though.

At the end of the method we call a service like this:

        return await this.post(`${ADS_URI}/bulks/import`, {
            body,
            headers: {
                ...buildHttpHeader({
                    ...context,
                    contentType: `multipart/form-data; boundary=${this.generateBoundary()}`,
                }),
            },
        });

generateBoundary is simply a copy of the functionality which form-data provided.

When initially trying to post without sending a Content-Type header explicitly, which worked before, we saw the 415 error described above.
After adding it, the service then complained about the categoryId information missing, which we append to the body though. This is the point where I’m still at. Makes me wonder if the post method has undergone some changes. But I can’t remember seeing anything in the migration guide.

8

Are you using the latest (newly-named package) @apollo/datasource-rest v5 or the previous package apollo-datasource-rest?

There’s a lot to recreate here, and I’m actually not familiar with file uploads myself. To set expectations, it’s unlikely I’ll find the time to try to reproduce this from scratch, so if you can put together a minimal reproduction we’ll have a good jumping off point to start helping you.

In the meantime, sharing your package.json (or all relevant dependencies) will be the absolute minimum for me to try to be helpful.

9

Thanks a lot for trying to help, @trevor.scheer. We’re using @apollo/datasource-rest but currently at version 4.3.2. Will try to update to 5 tomorrow. Here is the the current package.json (part with the deps):

{
    "dependencies": {
        "@apollo/datasource-rest": "^4.3.2",
        "@apollo/server": "^4.3.0",
        "@apollo/server-plugin-landing-page-graphql-playground": "^4.0.0",
        "@apollo/utils.fetcher": "^2.0.0",
        "@as-integrations/fastify": "^1.3.0",
        "@fastify/cors": "^8.2.0",
        "@graphql-tools/schema": "^8.5.0",
        "@graphql-tools/utils": "^8.8.0",
        "@newrelic/apollo-server-plugin": "^2.1.0",
        "@sentry/integrations": "^6.13.2",
        "@sentry/node": "^6.13.2",
        "dataloader": "^2.1.0",
        "dotenv": "latest",
        "fastify": "^4.13.0",
        "form-data": "^3.0.1",
        "graphql": "^16.6.0",
        "graphql-tag": "^2.12.6",
        "graphql-upload": "^16.0.2",
        "launchdarkly-node-server-sdk": "^7.0.0",
        "lodash": "^4.17.21",
        "newrelic": "^9.0.3",
        "query-string": "^7.1.3",
        "raw-body": "^2.5.2"
    },
    "devDependencies": {
        "@apollo/rover": "^0.10.0",
        "@babel/preset-env": "^7.20.2",
        "@swc/core": "^1.3.37",
        "@swc/jest": "^0.2.24",
        "@types/jest": "^29.2.5",
        "@types/lodash": "^4.14.175",
        "@types/newrelic": "^7.0.2",
        "@types/node": "18.11.18",
        "@typescript-eslint/eslint-plugin": "^4.33.0",
        "@typescript-eslint/parser": "^4.33.0",
        "babel-jest": "^29.4.1",
        "eslint": "^7.32.0",
        "eslint-plugin-unused-imports": "1.1.5",
        "graphql-type-json": "^0.3.2",
        "husky": "^8.0.2",
        "jest": "^29.3.1",
        "jest-sonar-reporter": "^2.0.0",
        "lint-staged": "^13.0.3",
        "nodemon": "^2.0.13",
        "prettier": "^2.8.1",
        "sonarqube-scanner": "^2.8.1",
        "ts-jest": "^29.0.3",
        "ts-node": "^10.9.1",
        "tsc-files": "^1.1.3",
        "typescript": "^4.9.4",
        "wait-on": "^7.0.1"
    }
}

Tomorrow I will think about how I can provide more.

12

Upgrading to @apollo/datasource-rest v5 does not make the issue go away unfortunately.

13

I looks like we were able to solve it. Was related to the change of how to pass params to the post method (2 instead of 3 params now). That caused the issue with the categoryId param not being recognized by the data source anymore.

14

Unfortunately, we were actually only able to partially fix the issue, @trevor.scheer. We now get the categoryId param through to the server successfully, but the files still don’t work.

We’re using the params property on the request object sent to the post method now (before that we just passed the FormData as the second argument to post). The thing is that everything in there has to be string now according to the type in the RestDataSource source code:

params?: URLSearchParamsInit;

So, this works for categoryId:

       return await this.post(`${ADS_URI}/bulks/import`, {
             params: new URLSearchParams([
                  ['categoryId', String(categoryId)],
             ]),
            headers: {
                ...buildHttpHeader({
                    ...context,
                    contentType: `multipart/form-data; boundary=${this.generateBoundary()}`,
                }),
            },
        });

And even this seems to work:

       return await this.post(`${ADS_URI}/bulks/import`, {
             params: {
                  categoryId: String(categoryId),
             },
            headers: {
                ...buildHttpHeader({
                    ...context,
                    contentType: `multipart/form-data; boundary=${this.generateBoundary()}`,
                }),
            },
        });

But when I add the adsFile, in whatever stringified form, the server we call doesn’t recognize it. It’s written in Kotlin and expects an adsFile of type MultipartFile, i.e. a string won’t work. So, to me it looks like we would have to rewrite the server code to be able to upgrade to Apollo Server 4.

15

See @apollo/datasource-rest and multipart file uploads