cloudflare / chanfana

OpenAPI 3 and 3.1 schema generator and validator for Hono, itty-router and more!
https://chanfana.pages.dev
MIT License
301 stars 40 forks source link

Swagger UI example request not being properly executed if mounted at /some/path #150

Closed carafelix closed 4 months ago

carafelix commented 4 months ago

If, for example, you mount itty-router-openapi as a child endpoint inside a hono application, for e.g:

import { OpenAPIRouter } from '@cloudflare/itty-router-openapi'
import { Hono } from 'hono'
// import exampleFetch etc

const router = OpenAPIRouter({
  docs_url: "/doc",
})
router.get('/example', ExampleFetch)

const app = new Hono()
app.mount('/api', router.handle)

Request to the /api/example would succeed, but running the examples from the swagger ui mounted at /api/doc would result in misrouted errors. Adding api as basepath to the OpenAPIRouter would only trail the error.

A solution could be to add a execution_basePath property like

const router = OpenAPIRouter({
  docs: {url: "/doc", execution_basePath: "/api"}
})

Ofc this can be worked around by mounting on / and adding the /api as a basepath in the itty router, but not as ideomatic

G4brym commented 4 months ago

This happens due to an implementation detail on Honojs (docs here), basically they "manipulate" the request url before calling the inner mounted router, for the inner router to think they are at the root of the request.

TLDR: /api/docs renders correctly, but points to a non-existing /openapi.json schema that is actually located at /api/openapi.json

I will think in a way to fix this issue, but currently the workaround is to disable the docs_url option, and to build your own docs page pointing to the correct path, then telling openapi that all routes should be prefixed with the base url.

Here's a working example:

import { OpenAPIRouter, getSwaggerUI } from '@cloudflare/itty-router-openapi'
import { Hono } from 'hono'

const router = OpenAPIRouter({
  docs_url: null,
  schema: {
    servers: [{
      url: '/api',
    }],
  },
})
router.get('/test', (request) => new Response(request.url))

router.original.get('/docs', () => {
  return new Response(
    getSwaggerUI('/api/openapi.json'),
    {
      headers: {
        'content-type': 'text/html; charset=UTF-8',
      },
      status: 200,
    },
  )
})

const app = new Hono()
app.mount('/api', router.handle)

export default app
carafelix commented 4 months ago

Just as additional info. It's possible to set basepath to /api and hono.mount('/', router.handle) Everything would work fine, but it's not very ideomatic and every request would pass through the openAPIrouter first

G4brym commented 4 months ago

Hello @carafelix in the latest version of Hono this issue is solved, all you need to do is set the base parameter in itty-router-openapi and pass this function when mounting the router in Hono:

app.mount('/api', router.handle, {
  replaceRequest: (req) => req,
})

Here is a full working example running in Hono v4.4.3

import { OpenAPIRoute, OpenAPIRouter } from '@cloudflare/itty-router-openapi'
import { Hono } from 'hono'

export class ToDoCreate extends OpenAPIRoute {
  static schema = {
    tags: ['ToDo'],
    summary: 'Create a ToDo',
  }

  async handle(
    request: Request,
    env: any,
    context: any,
    data: any,
  ) {
    return Response.json({ success: true })
  }
}

const router = OpenAPIRouter({
  base: '/api',
})
router.get('/example', ToDoCreate)

const app = new Hono()
app.mount('/api', router.handle, {
  replaceRequest: (req) => req,
})

export default app

Then openning the browser at http://localhost:8787/api/docs shows the correct swagger ui

carafelix commented 4 months ago

Wow that was fast! Thank you very much. I have noticed other things in the router. like not supporting Authorization headers and such. Should I open an issue? I think on taking a look at the inner workings of the router in the near future, so I could help with development, like getting better type inferences and such. Do you have a todo list I can look at?

G4brym commented 4 months ago

I'm already working on a very big revamp for this library, that will make it "router" agnostic. This is a big push to make this work natively on Hono You can see a preview of the changes on this pull request #151 I already have much better type inference working, and a lot more improvements done

G4brym commented 4 months ago

I'm planning on releasing this new revamp version in the next weeks, so after that i'm happy to review and discuss additional changes 😄