Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple sub-apps mounted on the same route result in Not Found for further subapps #279

Closed
talentlessguy opened this issue Jun 24, 2021 · 5 comments · Fixed by #329
Closed
Labels
bug Something isn't working core Affects `@tinyhttp/app` - the tinyhttp core good first issue Good for newcomers help wanted Extra attention is needed

Comments

@talentlessguy
Copy link
Member

talentlessguy commented Jun 24, 2021

Describe the bug

When you mount two apps on the same route only the first one works and the other doesn't get reached.

To Reproduce

import { App } from '@tinyhttp/app'

const app = new App(), app1 = new App(), app2 = new App()

app2.use((req, res, next) => {
	console.log('ping')
	res.send('hi from app 2')
})

app1.use((req, res, next) => {
	console.log('pong')
	next()
})

app.use('/test', app1)
app.use('/test', app2)


app.listen(3000)
$ curl localhost:3000/test
Not Found

Expected behavior

if not found it should go to the next app and send "hi from app 2"

Versions

  • node: 16.3
  • @tinyhttp/app: 1.3.11

Additional context

#276

@talentlessguy talentlessguy added bug Something isn't working core Affects `@tinyhttp/app` - the tinyhttp core labels Jun 24, 2021
@lucascebertin
Copy link

Is that issue related to this piece of code and the apps: Record<string, App>?

    for (const fn of fns) {
      if (fn instanceof App) {
        regex = rg(path, true)

        fn.mountpath = path

        this.apps[path] = fn // when paths collide, only the last one will be considered and executed 

        fn.parent = this
      }
    }

@talentlessguy
Copy link
Member Author

@lucascebertin I am not sure but yeah either here in App.use or in Router.use

@talentlessguy talentlessguy added good first issue Good for newcomers help wanted Extra attention is needed labels Sep 7, 2021
@KaviiSuri
Copy link

Hey, I'd love to give this one a shot too... I'm new to open source and tinyhttp and was hoping for some guidance

@talentlessguy
Copy link
Member Author

@KaviiSuri there are setup instructions in CONTRIBUTING.md but for this bug you should be looking into app and router packages

to test your code I'd recommend creating a unit test in tests/core/app.test.ts and there write a test case with expected behavior

@Doc999tor
Copy link

I'm facing the same issue and prepared even smaller and simpler reproducible example
http://localhost:3000/ping - works correctly
http://localhost:3000/pong - returns 404 no matter what

import { App } from '@tinyhttp/app'
const app = new App(), app1 = new App(), app2 = new App()

app1.get('/ping', (req, res, next) => {
	res.send('ping')
})

app2.get('/pong', (req, res, next) => {
	res.send('pong')
})

app.use(app1)
app.use(app2)

app.listen(3000)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core Affects `@tinyhttp/app` - the tinyhttp core good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants