2.0 seems to have broken static file serving on azure.
CyrusNajmabadi opened this issue · 12 comments
I have the following code in my function-app:
app.use("/", express.static("www"));This results in the following runtime exception:
2018-09-07T22:14:29.829 [Info] Function started (Id=65d1f81f-f1ed-4d9f-844e-7fdcae8cf5c6)
2018-09-07T22:14:31.782 [Error] A ScriptHost error has occurred
2018-09-07T22:14:31.782 [Error] System.InvalidOperationException : TypeError: Cannot read property 'length' of null
at ServerResponse.OutgoingMessage._send (_http_outgoing.js:137:38)
at ServerResponse.OutgoingMessage.write (_http_outgoing.js:491:16)
at ReadStream.ondata (_stream_readable.js:555:20)
at emitOne (events.js:96:13)
at ReadStream.emit (events.js:188:7)
at readableAddChunk (_stream_readable.js:176:18)
at ReadStream.Readable.push (_stream_readable.js:134:10)
at onread (fs.js:2018:12)
at FSReqWrap.wrapper [as oncomplete] (fs.js:682:17)
Note the calls into the new methods inherited by OutgoingMessage.
I'm not sure what's up here but i think this starting breaking when the change to derive from NativeOutgoingMessage. Do you know what's up @yvele ? Do you have a recommendation on what can be used instead?
The crashing line in question appears to be: https://github.com/nodejs/node/blob/1cb5aed5f596a9cb5b68e3a676222c0c35501a5e/lib/_http_outgoing.js#L137
} else {
this.output.unshift(this._header);
this.outputEncodings.unshift('latin1');
this.outputCallbacks.unshift(null);
this.outputSize += this._header.length; //<-- this line.
if (typeof this._onPendingData === 'function')
this._onPendingData(this._header.length);
}@yvele I've been trying my best to debug through this and figure out what the right thing to do is. but i keep hitting a brick wall. The node http stack is just insanely complicated. I've tried as well on 10.6 but this is broken there as well.
If you could help out here at all it would be very appreciated. Thanks!
@maktwin Do you have any ideas either? Do either of you two use static file serving in your FunctionApps? If so, do you do anything special to make it work?
Diagnostic
I've overwritten the internal writeHead method to setup context.res.headers (specific to Azure Functions) but it's specific to node 6.
azure-function-express/src/OutgoingMessage.js
Lines 35 to 45 in 2bd9041
Node 8~10 doesn't have this internal writeHead method anymore 🤔
PS: By the way this may be an inspiration https://github.com/awslabs/aws-serverless-express
Possible solution
By inheriting native OutgoingMessage we maybe can skip manually implementing writeHead and maybe we can do something like:
constructor(context) {
super();
this.context = context; // Azure Function context
}
// Override by adding things specific to Azure Functions runtime
end(data, encoding) {
super.end(data, encoding);
// Return raw values to Azure Function runtime
const { context } = this;
context.res.headers = ...;
context.res.status = ...;
context.res.body = convertToBody(data, encoding);
context.res.isRaw = true;
context.done();
}We also need unit tests for that (covering all supported node versions).
Interesting info. Note: i've been using aws-serverless-express very successfully on aws itself. I was going to try something today which first seemed odd, but now i think would b ea good idea. Specifically, i was going to just trying using aws-serverless-express on azure. It's just a library which spawns a local http server, then sends an http request to it. It does expect to take an APIGatewayRequest+serverless.Context as arguments, and it will return produce different result values than azure needs. However, i think i can map azure's inputs to the inputs it needs, and i think i can map its outputs to the ones that azure needs as well.
By actually talking to a real node http server, there is no need to try to simulate an OutgoingMessage/ServerResponse. Instead, the true node will be used. That should hopefully make it so all middleware (like express.static) works fine. All that needs to happen is to properly marshal between the azure and aws input/output types, which i think is very doable...
I got this all working by moving over to aws-serverless-express. Turns out it works fine as a libary on azure. The code to get it to work was pretty simple, and i can link to it once it's cleaned up.
PR that switches to just using aws-serverless-express: pulumi/pulumi-cloud#584
We're also considering extracting this into a standalone library . Let me know if you'd be interested in that!
That's for the tip @CyrusNajmabadi , I found a different library that implemented aws-serverless-express for azure and it worked like a charm (https://github.com/bigx333/azure-aws-serverless-express).
@yvele I do really like this library, and would switch back once this issue gets fixed. To give you more details, I was trying to serve a ReadableStream from azure blob storage through express and kept running into the headers issue.
maybe it's related in "OutgoingMessage.js"
I changed "OutgoingMessage.js" like below
function convertToBody(body, encoding) {
// return Buffer.isBuffer(body) ? body.toString(encoding) : body; //origin code
return Buffer.isBuffer(body) && "binary" != encoding ? body.toString(encoding) : body;
}
and test like this
app.get("/api/getfile", function (req, res) {
const data = fs.readFileSync("sample.pdf");
res.writeHead(200, {
"Content-Type": "application/pdf",
"Content-Disposition": "attachment;filename=output.pdf"
});
res.end(data, "binary");
});
It downloaded the file without "TypeError: Cannot read property 'length' of null"
Same problem, checking the encoding like @kfhak-zb does solves the issue
any update on this ?
Anyone get this working?