preSerialization are not executed when content-type is set

[Eomm]

Prerequisites

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

Fastify version

latest

Plugin version

No response

Node.js version

20

Operating system

Linux

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

Description

The preSerialization hooks do not run when the content-type is customized:

Attempted to send payload of invalid type 'object'. Expected a string or Buffer

Steps to Reproduce

const app = require('fastify')({ logger: true });

app.addHook('preSerialization', (request, reply, payload, done) => {
  if (reply.getHeader('content-type') === 'application/xml') {
    done(null, payload.toXmlString());
  } else {
    done(null, payload);
  }
});

app.get('/', async (request, reply) => {
  reply.type('application/xml');

  return {
    toXmlString () {
      return 'hello';
    },
  };
});

app.inject('/');

Expected Behavior

I think we should run the preSerialization hooks when a custom content-type is set.
Do we force the user to write?

app.get('/', async (request, reply) => {
  reply
    .type('application/xml') //
    .serializer((data) => {
      return data.toXmlString();
    });

  return {
    toXmlString () {
      return 'hello';
    },
  };
});

WDYT?

[jsumners]

If I'm reading things correctly, the reproduction should hit line 189 in:

fastify/lib/reply.js

Lines 162 to 215 in ac26d7b

	const contentType = this.getHeader('content-type')
	const hasContentType = contentType !== undefined
	if (payload !== null) {
	if (typeof payload.pipe === 'function') {
	onSendHook(this, payload)
	return this
	}
	if (payload?.buffer instanceof ArrayBuffer) {
	if (hasContentType === false) {
	this[kReplyHeaders]['content-type'] = CONTENT_TYPE.OCTET
	}
	const payloadToSend = Buffer.isBuffer(payload) ? payload : Buffer.from(payload.buffer, payload.byteOffset, payload.byteLength)
	onSendHook(this, payloadToSend)
	return this
	}
	if (hasContentType === false && typeof payload === 'string') {
	this[kReplyHeaders]['content-type'] = CONTENT_TYPE.PLAIN
	onSendHook(this, payload)
	return this
	}
	}
	if (this[kReplySerializer] !== null) {
	if (typeof payload !== 'string') {
	preSerializationHook(this, payload)
	return this
	} else {
	payload = this[kReplySerializer](payload)
	}
	// The indexOf below also matches custom json mimetypes such as 'application/hal+json' or 'application/ld+json'
	} else if (hasContentType === false || contentType.indexOf('json') > -1) {
	if (hasContentType === false) {
	this[kReplyHeaders]['content-type'] = CONTENT_TYPE.JSON
	} else {
	// If user doesn't set charset, we will set charset to utf-8
	if (contentType.indexOf('charset') === -1) {
	const customContentType = contentType.trim()
	if (customContentType.endsWith(';')) {
	// custom content-type is ended with ';'
	this[kReplyHeaders]['content-type'] = `${customContentType} charset=utf-8`
	} else {
	this[kReplyHeaders]['content-type'] = `${customContentType}; charset=utf-8`
	}
	}
	}
	if (typeof payload !== 'string') {
	preSerializationHook(this, payload)
	return this
	}
	}

If it's not, I'd classify that as a bug.

[Eomm]

Nope, it goes to:

fastify/lib/reply.js

Line 217 in ac26d7b

onSendHook(this, payload)

[jsumners]

That seems very wrong.

[climba03003]

That seems very wrong.

How can you expect to have preSerialization when no serializer is actually exists?
If no serialize is on action, I don't think we should expect preSerialization to run.

[Eomm]

How can you expect to have preSerialization when no serializer is actually exists?

I was expecting it to be executed since I did not provide any response that is already serialized. We are blocking the possibility of configuring a hook to work like a serializer.

I would write down that the preSerializer hooks run only if:

• the content type is a */json
• the content type is not set
• the payload is not already serialized

Anyway, should we improve the usability?
The reply.serializer() one doesn't look good compared to other features we have.

I'm thinking something like:

• addContentTypeSerializer(contentType, serializerFn) ---- Like addContentTypeParser()
• Every serializer could have a dedicated preSerialization hook array (?)

[climba03003]

I was expecting it to be executed since I did not provide any response that is already serialized.

preSerializer is a hook, not a serializer. So, the expectation is wrong in first place.

Anyway, should we improve the usability?

What I come in mind is we should do something like addContentTypeParser (as you said).

fastify.addContentTypeSerializer('text/xml', function (reply, payload, done) {
  done('')
})

Better support would be addContentType. Then we can deprecate addContentTypeParser and do it in central management.

fastify.addContentType('text/xml', {
  // output for reply
  serialize: (reply, payload, done) => {},
  // input for request
  deserialize: (request, payload, done) => {}
})

[msebastianb]

I'd try to implement something for this, if no one is working on it.

[mcollina]

@Eomm There was never the intention to call that hook in this situation because it lead to having a gigantic if based on the content type.