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

addContentTypeParser doesn't work well after await register #5401

Open
2 tasks done
scott1028 opened this issue Apr 13, 2024 · 5 comments
Open
2 tasks done

addContentTypeParser doesn't work well after await register #5401

scott1028 opened this issue Apr 13, 2024 · 5 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@scott1028
Copy link

scott1028 commented Apr 13, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

v4.26.2

Plugin version

No response

Node.js version

20

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

20.04

Description

addContentTypeParser doesn't work as expected when previous statement is register with await.

Steps to Reproduce

ex:

'use strict'

const main = async () => {
  const server = require('fastify')({
    logger: true,
  });

  const schema = {
    schema: {
      response: {
        200: {
          type: 'object',
          properties: {
            hello: {
              type: 'string',
            },
          },
        },
      },
    },
  };

  await server.register((fastify, options, done) => {
    fastify.get('/api/upload', async (req, res) => {
      return { hello: 'world0' };
    })
    fastify.post('/api/upload', async (req, res) => {
      return { hello: 'world1' };
    })
    done();
  });

  server.get('/', schema, function (req, res) {
    res.send({ hello: 'world2' });
  });

  // This parser will not work after "server.register" in "await"
  server.addContentTypeParser(
    'text/csv',
    { parseAs: 'string' },
    (req, body, done) => {
      // Custom parsing logic for text/csv content type for api plugin
      const csvData = body.split('\n').map((row) => row.split(','));
      done(null, csvData);
    },
  );

  server.listen({ port: 3000 }, (err, address) => {
    if (err) {
      throw err;
    }
  });
};

main();
curl -H "Content-Type: text/csv" -X POST --data "a,b,c" http://localhost:3000/api/upload
// you will get below
{"statusCode":415,"code":"FST_ERR_CTP_INVALID_MEDIA_TYPE","error":"Unsupported Media Type","message":"Unsupported Media Type: text/csv"}

Expected Behavior

the CSV parser should work as expected.

curl -H "Content-Type: text/csv" -X POST --data "a,b,c" http://localhost:3000/api/upload
{"hello":"world1"}
@scott1028 scott1028 changed the title addContentTypeParser doesn't work well after await register addContentTypeParser doesn't work well after await register Apr 13, 2024
@Uzlopak
Copy link
Contributor

Uzlopak commented Apr 13, 2024

So you claim there is a race condition?

@scott1028
Copy link
Author

scott1028 commented Apr 13, 2024

So you claim there is a race condition?

I don't think it a race condition issue because it can work by removing await from await server.register.
Also, I'm not sure if this was caused by avvio used by fastify or not.

@climba03003
Copy link
Member

climba03003 commented Apr 14, 2024

Not sure if is it mentioned in document.
But await fastify.register is the same as fastify.after which means all the later code will be after context finalize from that plugin.

My suggestion is that only add await when necessary.

@climba03003
Copy link
Member

I believe similar fix can be used like setErrorHandler.
See #4204 (comment) #4205

@mcollina
Copy link
Member

Albeit surprisingly, this is expected behavior. This should be clarified in our docs.

@mcollina mcollina added documentation Improvements or additions to documentation good first issue Good for newcomers labels Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants