-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.0 seems to have broken static file serving on azure. #21
Comments
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? |
DiagnosticI've overwritten the internal azure-function-express/src/OutgoingMessage.js Lines 35 to 45 in 2bd9041
Node 8~10 doesn't have this internal PS: By the way this may be an inspiration https://github.com/awslabs/aws-serverless-express Possible solutionBy inheriting native 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 @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 |
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 } and test like this app.get("/api/getfile", function (req, res) {
}); 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? |
I have the following code in my function-app:
This results in the following runtime exception:
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 text was updated successfully, but these errors were encountered: