Skip to content

Commit

Permalink
refactor(hooks)!: Async hooks must return false when sending a response
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Async hooks that send a response must now return `false`. If a hook sends a response but does not return `false`, the next hooks and handler will be run, and res.send() will probably be called twice, resulting in an error.
  • Loading branch information
nwoltman committed Jun 24, 2019
1 parent b04a8ea commit 2a51d74
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 10 deletions.
20 changes: 13 additions & 7 deletions docs/Hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,15 @@ app.addHook('onRequest', (req, res, next) => {

If sending a response from inside a hook, **`next()` must not be called**.

If sending a response from inside an `async` hook, **`res.send()` must be used**.
If it is not, Medley will not know that a response has been sent, so the hooks
will continue running.
If sending a response from inside an `async` hook, **the hook must return
`false`** to prevent Medley from continuing on to run the next hooks.

```js
app.addHook('onRequest', async (req, res) => {
res.send({ early: 'response' });
return false;
});
```

### Handling Errors

Expand All @@ -105,19 +111,19 @@ app.addHook('onRequest', (req, res, next) => {
next(err);
return;
}
// ...
});
});
```

Async-await/promise errors are automatically caught and handled by Medley.

```js
const fs = require('fs');
const util = require('util');
const readFile = util.promisify(fs.readFile);
const fs = require('fs').promises;

app.addHook('onRequest', async (req, res) => {
const buffer = await fs.readFile('./someFile.txt');
// ...
});
```

Expand Down Expand Up @@ -164,7 +170,7 @@ app.addHook('onSend', (req, res, payload, next) => {
});
```

To modify the payload using an `async` hook, return the new payload:
To modify the payload using an `async` hook, return the new payload.

```js
app.addHook('onSend', async (req, res, payload) => {
Expand Down
4 changes: 2 additions & 2 deletions lib/HookRunners.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ const HookRunners = {
}
}

function handleResolve() {
if (!res.sent) {
function handleResolve(value) {
if (value !== false) {
next(null)
}
}
Expand Down
1 change: 1 addition & 0 deletions test/hooks-async.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ test('onRequest hooks should be able to send a response', (t) => {

app.addHook('onRequest', async (req, res) => {
res.send('hello')
return false
})

app.addHook('onRequest', async () => {
Expand Down
2 changes: 1 addition & 1 deletion test/hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ test('async onRequest hooks should be able to send a response', (t) => {

app.addHook('onRequest', (req, res) => {
res.send('hello')
return Promise.resolve()
return Promise.resolve(false)
})

app.get('/', () => {
Expand Down

0 comments on commit 2a51d74

Please sign in to comment.