fix: referenceError on schema validation failure.

[LucaRainone]

this solves #163

Checklist

• run npm run test and npm run benchmark
• tests are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[gurgunday]

Not sure why this works – an async hook should not return because a return essentially means returning the body/payload

Can you instead wrap the try finally with a non-negated if?

    const onSend = async (req) => {
      const requestClient = extractRequestClient(req, transact)
      if (requestClient) {
        try {
          if (!req[transactionFailedSymbol]) {
            await requestClient.query('COMMIT')
          }
        } finally {
          requestClient.release()
        }
      }
    }

[LucaRainone]

Not sure why this works – an async hook should not return because a return essentially means returning the body/payload

Can you instead wrap the try finally with a non-negated if?

    const onSend = async (req) => {
      const requestClient = extractRequestClient(req, transact)
      if (requestClient) {
        try {
          if (!req[transactionFailedSymbol]) {
            await requestClient.query('COMMIT')
          }
        } finally {
          requestClient.release()
        }
      }
    }

AFAIK in JS a function returns undefined without explicit return statement (https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist see step 10). So return ; is equivalent to return undefined that's equivalent to complete the function without returning anything. And in this particular context an async function always returns a Promise, that will be resolved on nextTick with the undefined value.

In pratical:

async function foo() {}
foo().then((val)=>console.log({val}));
// {val: undefined}, Promise{<fulfilled>:undefined}

I don't know if this is the right line, but https://github.com/fastify/fastify/blob/fc6bf5cb9f000ad4cda5e197f4fd22166a15dab9/lib/hooks.js#L288 here you can see that there is a check on payload to be different to undefined. So, that's why it works based on my thoughts.

The proposal change could be still about code style (personally I prefer to avoid nested code, but it's only an opinion and honestly I didn't check the other parts of code): should I still change it?

[LucaRainone]

The proposal change could be still about code style (personally I prefer to avoid nested code, but it's only an opinion and honestly I didn't check the other parts of code): should I still change it?

Done in any case

[gurgunday]

Thank you!

You are right in your assessment of what's going on when undefined is returned inside an async function in the context of hooks

At the time, I simply thought since this is a core plugin it should mirror the usage shown in the docs

Excuse me for taking your time with this

[LucaRainone]

Thank you!

You are right in your assessment of what's going on when undefined is returned inside an async function in the context of hooks

At the time, I simply thought since this is a core plugin it should mirror the usage shown in the docs

Excuse me for taking your time with this

np: I like to go in deep and I had the opportunity to see other parts :) . Thank you for your review