Skip to content

feat: support async constraint#4323

Merged
mcollina merged 1 commit intomainfrom
async-constraints
Oct 10, 2022
Merged

feat: support async constraint#4323
mcollina merged 1 commit intomainfrom
async-constraints

Conversation

@climba03003
Copy link
Copy Markdown
Member

@climba03003 climba03003 commented Oct 6, 2022

Add support for async constraints

Checklist

@Uzlopak
Copy link
Copy Markdown
Contributor

Uzlopak commented Oct 6, 2022

Does hasRoute need also some love?

@climba03003
Copy link
Copy Markdown
Member Author

climba03003 commented Oct 6, 2022

Does hasRoute need also some love?

There is no API changes in hasRoute or find-my-way.find

@climba03003
Copy link
Copy Markdown
Member Author

CI failure due to Github Actions outage
https://www.githubstatus.com/incidents/gq1x0j8bv67v

I will re-run the CI after Github fix it.

@Uzlopak
Copy link
Copy Markdown
Contributor

Uzlopak commented Oct 6, 2022

Github Actions run again properly
Restarted the failed CI

Code coverage is low

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@climba03003 climba03003 requested a review from Fdawgs October 6, 2022 17:05
})
```

#### Asynchronous Custom Constraints
Copy link
Copy Markdown
Member Author

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?

Copy link
Copy Markdown
Member

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)
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Member

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)
}

Copy link
Copy Markdown
Member Author

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.

@github-actions
Copy link
Copy Markdown

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants