multer
multer copied to clipboard
fix: wrap middleware in promise
this is necessary, so that async hooks work
#814 #1046
tests all pass.
no new tests have been added
Thank you for the PR! Currently there are unfortunately two problems which means we cannot merge it yet:
- This is a breaking change, since Multer 1.x supports Node.js 0.10 which doesn't have
Promise - If
nextthrows an error that will be caught by us, and then we will callnextagain with that error. This is a very common misstake to make when trying to "callbackify" a promise. You can look here for some info on doing it the "correct" way: https://github.com/LinusU/util-callbackify/blob/master/index.js
Hi @LinusU do we have tests which check for your second point?
I wouldn't know how to fix the problem without promises. Should we close that PR then?
[...] do we have tests which check for your second point?
Not at the moment, but this would be a great to add!
I wouldn't know how to fix the problem without promises. Should we close that PR then?
One idea could be to only enable this path if typeof Promise === 'function'. I'm not to familiar with the context but I think that this can be solved without wrapping the entire thing in a Promise, instead using the async_hook module to track the context.
There seems to be some documentation here: https://nodejs.org/api/async_context.html
This could then be imported something like:
var asyncHook
try {
asyncHook = require('async_hook')
} catch (_) {}