jkyberneees / low-http-server

HTTP Server implementation in top of uWebSocket.js
MIT License
50 stars 7 forks source link

fix hang upload file in second request #13

Closed herudi closed 3 years ago

herudi commented 3 years ago

Hi @jkyberneees @schamberg97, i'm try upload file but hang on second request. I've changed and fixed a few lines of code. please correct it again.

This example upload

const multer = require('multer');
const { router, server } = require('0http')({
    server: require('../src/server')()
});

const upload = multer({ 
    storage: multer.diskStorage({
        destination: __dirname,
        filename: (req, file, cb) => cb(null, file.originalname)
    })
});

// try in second request will hang
router.post('/upload', upload.single('file'), (req, res) => {
    if (!req.file) {
        res.statusCode = 400;
        res.end('Failed to upload');
    }
    res.end('Success upload ' + req.file.originalname);
})

server.listen(3000, () => { })
schamberg97 commented 3 years ago

From what I understood with this commit, it partially alleviates the issue. I have tried it with both previous 2.0 version and the new 3.0 (after solving conflicts, obviously). It does fix the issue of hanging, but does not resolve the failure to upload file. It appears that for some reason the route in your demo triggers twice oO

schamberg97 commented 3 years ago

Adding this check right before const chunk or let chunk declaration also helps to avoid the crash:

if (!bytes.byteLength) {
          if (!isLast) return
          return handler(reqWrapper, resWrapper)
}

However, neither way helps to implement multipart/form-data file upload. Something is probably broken with HttpRequest class

herudi commented 3 years ago

at first I saw this error... this.slice is not define or not function.

before

_read (size) {
    return this.slice(0, size)
 }
schamberg97 commented 3 years ago

I propose we fix these issues together in https://github.com/jkyberneees/low-http-server/pull/14 , because they are made against the newer code base. If you agree, we can close this thread

herudi commented 3 years ago

i agree. +1