fastify / point-of-view

Template rendering plugin for Fastify
MIT License
344 stars 87 forks source link

Promise may not be fulfilled with 'undefined' when statusCode is not 204 #266

Closed yaneony closed 2 years ago

yaneony commented 3 years ago

Prerequisites

Fastify version

latest

Plugin version

latest

Node.js version

latest

Operating system

Windows

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

Windows 10-11 / Debian 10-11

Description

Following code was used (see below)

Open /1 in your browser and just hold on F5.. you will get a lot of errors in your console with following message: [1636309790914] ERROR (18208 on DESKTOP): Promise may not be fulfilled with 'undefined' when statusCode is not 204

While, doing same on /2 is working fine.

As per documentation, and also example from here https://github.com/fastify/point-of-view/blob/master/example-async.js#L25 it should be fine to reply.view without await, but it looks like you'll have to use await as well.

Is that expected behavior?

Steps to Reproduce

const fastify = require('fastify')({ trustProxy: true, logger: { prettyPrint: true }, disableRequestLogging: true, ignoreTrailingSlash: true });

fastify.register(require('point-of-view'), { engine: { ejs: require('ejs') }, viewExt: 'ejs' });

fastify.get('/1', async (req, res) => {
  return res.view('index');
});

fastify.get('/2', async (req, res) => {
  await res.view('index');
  return res;
});

fastify.listen(4020);

Expected Behavior

No response

Eomm commented 3 years ago

Is that expected behavior?

I don't think so

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

yaneony commented 2 years ago

I'm not sure what and where the problem is.

Uzlopak commented 2 years ago

Is returning actually correct? reply.view is a promise. So returning reply.view in the async await is an unresolved Promise, and as such "undefined".

So I think, you should remove the returns. It is also clarified in the docs. https://www.fastify.io/docs/latest/Routes/#promise-resolution

Eomm commented 2 years ago

I think the view method should return the res object

this will both code usages work

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.