[11.x]platform-fastify: custom methods should be registred

[johaven]

Did you read the migration guide?

• I have read the whole migration guide

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Potential Commit/PR that introduced the regression

No response

NestJS version

11.x

Describe the regression

This is a regression following the upgrade to version 5 of fastify, the SEARCH method and the new methods implemented need to be registered during the route injection phase (https://fastify.dev/docs/v5.1.x/Guides/Migration-Guide-V5/#removal-of-some-non-standard-http-methods)

instance.route(routeToInjectWithOptions) returns the following error:

FastifyError [Error]: SEARCH method is not supported.

Minimum reproduction code

No response

Input code

No response

Expected behavior

Custom methods should be registered automatically.

Other

I wanted to do a PR, the fix is ​​pretty simple, just check if the method is supported, in the injectRouteOptions function:

if (this.instance.supportedMethods.indexOf(routerMethodKey) > -1) {
    this.instance.addHttpMethod(routerMethodKey)
}

But I'm stuck on how to determine if the handler has a @Body decorator to use the hasBody option during the injection:

this.instance.addHttpMethod(routerMethodKey, { hasBody: true })

The other problem is to handle the case of multiple routes that may or may not have a body for the same custom method.

[kamilmysliwiec]

Instead of checking whether a route handler has a @Body decorator, I'd say we could determine IF a given HTTP method should allow a body or not based on its spec, e.g., SEARCH allows passing a body, so let's just set hasBody: true for it etc

[johaven]

For custom methods, having the choice on the use of a body remains a nice option. For example, COPY/MOVE methods are used in some contexts with and without a body.

Allowing the default body for these methods would be more generic and less restrictive (and that was the case until now.)

[kamilmysliwiec]

@johaven you won't be able to determine whether a method is annotated or not in this context (the adapter receives a wrapped proxy callback with no metadata attached)

[johaven]

@kamilmysliwiec This is what I noticed when trying to retrieve the ROUTE_ARGS_METADATA :(

Are you ok with my suggestion to set hasBody to true for these methods by default?

[kamilmysliwiec]

Sounds good!

[johaven]

There is another problem, this one concerns the decorator @ALL
This kind of endpoint returns a 404 if the methods are not declared in the fastify instance.
No fastify error is generated since the methods are not declared with a specific handler.

Example:

@All(WDAV_PATH)
  async files(@Req() req: FastifyDAVRequest, @Res({ passthrough: true }) res: FastifyReply) {
    switch (req.method) {
      case HTTP_METHOD.PROPFIND:
        return this.webdavMethods.propfind(req, res, SPACE_REPOSITORY.FILES)
      ....
      case HTTP_METHOD.COPY:
      case HTTP_METHOD.MOVE:
        return this.webdavMethods.copyMove(req, res)
      default:
        return res.status(HttpStatus.METHOD_NOT_ALLOWED).send()
    }
  }

@kamilmysliwiec If the @ALL decorator is to literally allow access to all methods, shouldn't we declare all non-standard methods on the fastify instance during initialization?

[kamilmysliwiec]

@kamilmysliwiec If the https://github.com/ALL decorator is to literally allow access to all methods, shouldn't we declare all non-standard methods on the fastify instance during initialization?

I'm not sure if anyone willing to use MOVE or COPY would do so through the ALL decorator. We can update the docs and clarify that ALL registers routes for all "original" methods (those that are available on the adapter out-of-the-box) and not for custom methods.

[kamilmysliwiec]

Let's track this here #14511