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
feat: support async constraint #4323
Conversation
Does hasRoute need also some love? |
There is no API changes in |
CI failure due to Github Actions outage I will re-run the CI after Github fix it. |
Github Actions run again properly Code coverage is low |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -732,6 +732,56 @@ fastify.route({ | |||
}) | |||
``` | |||
|
|||
#### Asynchronous Custom Constraints |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fdawgs Do you have time to helps me check the document changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @climba03003, missed this. Will take a look next week.
'Content-Type': 'application/json', | ||
'Content-Length': body.length | ||
}) | ||
res.end(body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you evaluate res.destroy
instead?
The only difference is whether or not emit an error
from the request object. Can't recall any other differences
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite understand what you means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed to refresh this
const { fastify } = require('fastify')
const app = fastify()
app.get('/destroy', async (request, reply) => {
reply.hijack()
reply.raw.destroy('hello world')
})
app.get('/end', async (request, reply) => {
reply.hijack()
reply.raw.end('hello world')
})
app.inject('/end', check)
.then(() => app.inject('/destroy', check))
function check (err, res) {
console.log(err)
console.log(res?.statusCode)
console.log(res?.payload)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.destory
cannot and will not send any payload to the client. Instead, it close the socket immediately.
.end
return a proper error message to the client and they can deal with it other than just wait for browser socket timeout.
219de22
to
c7519ea
Compare
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Add support for async constraints
Checklist
npm run test
andnpm run benchmark
and the Code of conduct