Skip to content
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

Be able to overwrite http response in IFunctionsWorkerMiddleware #530

Closed
Socolin opened this issue Jul 8, 2021 · 26 comments · Fixed by #814
Closed

Be able to overwrite http response in IFunctionsWorkerMiddleware #530

Socolin opened this issue Jul 8, 2021 · 26 comments · Fixed by #814
Assignees
Labels

Comments

@Socolin
Copy link

Socolin commented Jul 8, 2021

Hello,

In ASP.NET Core I have a middleware that try/catch around the next(context) and depending on the exception, set response body and set status code

try
{
	await _next(context);
}
catch (Exception exception)
{
	var errorResponse = _exceptionHandler.Handle(exception, context.RequestAborted.IsCancellationRequested);
	context.Response.StatusCode = errorResponse.StatusCode;
	context.Response.ContentType = "application/json";
	var jsonErrorResponse = JsonSerialize(errorResponse.Body);
	await context.Response.WriteAsync(jsonErrorResponse);
}

I would like to do the same in azure function in IFunctionsWorkerMiddleware but it's not possible currently, the FunctionContext does not explose anything to do that.

@ghost ghost assigned brettsam Jul 8, 2021
@brettsam
Copy link
Member

I believe this should be possible, but it may not be straightforward -- we'll investigate this and add a sample if it's possible today. Otherwise, we'll expose this.

@kzryzstof
Copy link

kzryzstof commented Jul 14, 2021

I asked a similar question on SO; that would be really nice if the use case was supported.

@david-peden-q2
Copy link

#340

@fabiocav
Copy link
Member

fabiocav commented Aug 4, 2021

@brettsam is still tracking this, but we'll need to push this to the next sprint. We'll continue to use this issue for updates.

@OlegGavrilov
Copy link

I believe this should be possible, but it may not be straightforward -- we'll investigate this and add a sample if it's possible today. Otherwise, we'll expose this.

Would really appreciate an update on this issue, thanks!

@Socolin
Copy link
Author

Socolin commented Sep 3, 2021

I believe this should be possible, but it may not be straightforward -- we'll investigate this and add a sample if it's possible today. Otherwise, we'll expose this.

Would really appreciate an update on this issue, thanks!

Same here, I'm waiting after this to improve errors message for our customers, be able to returns 401 / 403 with an error message can do a really big change from a generic 500.

@fabiocav fabiocav added the design label Sep 8, 2021
@fabiocav fabiocav removed this from the Functions Sprint 107 milestone Sep 8, 2021
@BrandonSchreck
Copy link

A bit hacky, but I was able to achieve this by setting up a couple extensions methods and a class that inherited from IFunctionsWorkerMiddleware:

/// <summary>
/// Returns the HttpRequestData from the function context if it exists.
/// </summary>
/// <param name="functionContext"></param>
/// <returns>HttpRequestData or null</returns>
public static HttpRequestData GetHttpRequestData(this FunctionContext functionContext, ILogger<ExceptionHandlerMiddleware> logger)
{
	try
	{
		object functionBindingsFeature = functionContext.GetIFunctionBindingsFeature(logger);
		Type type = functionBindingsFeature.GetType();
		var inputData = type?.GetProperties().Single(p => p.Name is "InputData").GetValue(functionBindingsFeature) as IReadOnlyDictionary<string, object>;
		return inputData?.Values.SingleOrDefault(o => o is HttpRequestData) as HttpRequestData;
	}
	catch (Exception ex)
	{
		logger.LogError(
			new EventId((int)LoggingEvents.GetHttpRequestDataFailure),
			ex,
			LoggingConstants.GetHttpRequestDataFailureTemplate
		);
		return null;
	}
}

/// <summary>
/// Sets the FunctionContext IFunctionBindingsFeature InvocationResult with a HttpResponseData. 
/// 
/// We are using this to add exceptions to the Azure Function response...to gently handle exceptions
/// caught by the exception handling middleware and return either a BadRequest 404 or Internal Server
/// Error 500 HTTP Result.
/// </summary>
/// <param name="functionContext"></param>
/// <param name="response"></param>
public static void SetHttpResponseData(this FunctionContext functionContext, HttpResponseData response, ILogger<ExceptionHandlerMiddleware> logger)
{
	try
	{
		object functionBindingsFeature = functionContext.GetIFunctionBindingsFeature(logger);
		Type type = functionBindingsFeature.GetType();
		PropertyInfo pinfo = type?.GetProperties().Single(p => p.Name is "InvocationResult");
		pinfo?.SetValue(functionBindingsFeature, response);
	}
	catch (Exception ex)
	{
		logger.LogError(
			new EventId((int)LoggingEvents.SetHttpResponseDataFailure),
			ex,
			LoggingConstants.SetHttpResponseDataFailureTemplate
		);
	}
}

/// <summary>
/// Retrieves the IFunctionBindingsFeature property from the FunctionContext.
/// </summary>
/// <param name="functionContext"></param>
/// <returns>IFunctionBindingsFeature or null</returns>
private static object GetIFunctionBindingsFeature(this FunctionContext functionContext, ILogger<ExceptionHandlerMiddleware> logger)
{
	try
	{
		KeyValuePair<Type, object> keyValuePair = functionContext.Features.SingleOrDefault(f => f.Key.Name is "IFunctionBindingsFeature");
		return keyValuePair.Value;
	}
	catch (Exception ex)
	{
		logger.LogError(
			new EventId((int)LoggingEvents.GetIFunctionBindingsFeatureFailure),
			ex,
			LoggingConstants.GetIFunctionBindingsFeatureFailureTemplate
		);
		return null;
	}
}
public class ExceptionHandlerMiddleware : IFunctionsWorkerMiddleware
{
	/// <summary>
	/// Gracefully catches, logs, and handles all exceptions from the Azure Functions.
	/// </summary>
	/// <param name="context"></param>
	/// <param name="next"></param>
	/// <returns></returns>
	public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next)
	{
		ILogger<ExceptionHandlerMiddleware> logger = context.GetLogger<ExceptionHandlerMiddleware>();

		try
		{
			await next(context);
		}
		catch (SomeCustomException ex)
		{
			logger.LogError(
				new EventId((int)LoggingEvents.SomeCustomException),
				ex,
				LoggingConstants.SomeCustomException
			);
			await SetResponse(context, logger, HttpStatusCode.BadRequest, ex);
		}
		catch (Exception ex)
		{
			logger.LogError(
				new EventId((int)LoggingEvents.InternalServerError),
				ex,
				LoggingConstants.InternalErrorTemplate,
				ex.Message
			);
			await SetResponse(context, logger, HttpStatusCode.InternalServerError, ex);
		}
	}

	private async Task SetResponse(FunctionContext context, ILogger<ExceptionHandlerMiddleware> logger, HttpStatusCode statusCode, Exception ex)
	{
		HttpRequestData req = context.GetHttpRequestData(logger);
		HttpResponseData response = await req?.ToResponse(statusCode, ex.Message);
		context.SetHttpResponseData(response, logger);
	}
}

@fabiocav
Copy link
Member

Assigning this to sprint 113 for initial design.

We'll share more information here once that is done.

@fabiocav
Copy link
Member

Work on this is still in progress. Updating sprint assignment

@frourke
Copy link

frourke commented Jan 13, 2022

When your function has multiple outputs you also need to add the HttpResponseData to the OutputBindingData dictionary to get the correct http response in your client.
Without this I was receiving a 200 OK with the error HttpResponseData serialized in the body.
`

    internal static void InvokeResult(FunctionContext context, HttpResponseData response)
    {
        var keyValuePair = context.Features.SingleOrDefault(f => f.Key.Name == "IFunctionBindingsFeature");
        var functionBindingsFeature = keyValuePair.Value;
        var bindingFeatureType = functionBindingsFeature.GetType();

        var result = bindingFeatureType.GetProperties().Single(p => p.Name == "InvocationResult");
        result?.SetValue(functionBindingsFeature, response);

        // handle HttpResponseData when a http triggered function has multiple outputs
        var outputBindingsInfo = bindingFeatureType
            .GetProperty("OutputBindingsInfo")
            .GetValue(functionBindingsFeature);
        var outputTypes = (List<string>)outputBindingsInfo
            .GetType()
            .GetField("_propertyNames", BindingFlags.NonPublic | BindingFlags.Instance)
            .GetValue(outputBindingsInfo);
        if (outputTypes?.Contains("HttpResponseData")) {
            var outputBindings = (Dictionary<string, object>)bindingFeatureType
                  .GetProperty("OutputBindingData")
                  .GetValue(functionBindingsFeature);
            outputBindings.Add("HttpResponseData", response);
        }
    }

`

@fabiocav
Copy link
Member

@frourke indeed. Those are the kinds of things we want to abstract away and make we have simple APIs that will do the right thing for you.

@RomanAlberdaSoftserveInc

Is this still in progress?

@kshyju kshyju assigned kshyju and unassigned fabiocav Feb 17, 2022
@kshyju
Copy link
Member

kshyju commented Feb 17, 2022

@RomanAlberdaSoftserveInc Yes, we are currently working on this item. The new APIs to support reading & altering input and output data from middleware will be available in the next few weeks. I will share an update to this thread as we make progress. Thanks!

@fabiocav
Copy link
Member

fabiocav commented Mar 2, 2022

Moving to sprint 118 as work is still in progress.

@fabiocav
Copy link
Member

PR for this is out and being reviewed (#814)

@mauleb
Copy link

mauleb commented Apr 14, 2022

Howdy @fabiocav, I see there were periodic releases until about 5 months ago. Forgive me if this is published somewhere I'm not seeing but when do you anticipate the next version will be pushed out with the changes described/introduced here and in #814?

@kshyju
Copy link
Member

kshyju commented Apr 14, 2022

@mauleb We plan to release next week. I will update this thread when the packages are published. Thanks!

@SeanFeldman
Copy link
Contributor

@kshyju would documentation get some love as well?

@kshyju
Copy link
Member

kshyju commented Apr 14, 2022

@kshyju would documentation get some love as well?

Absolutely. That is the plan.

@SeanFeldman
Copy link
Contributor

@kshyju following up on the documentation 🙂

@Arash-Sabet
Copy link

@kshyju Just wondering, is the documentation ready? Or, is there a code example on github to refer to?

@velocitysystems
Copy link

@kshyju Do you have any update on when this change will be released?

@kshyju
Copy link
Member

kshyju commented May 8, 2022

Version 1.8.0-preview1 of Microsoft.Azure.Functions.Worker package introduces a few new APIs which can be used to read & update input binding, output binding and invocation result data.

Here is an example of updating the http response inside a middleware.

public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next)
{
    try
    {
        await next(context);
    }
    catch (Exception ex)
    {
        _logger.LogError(ex, "Error processing invocation");

        var httpReqData = await context.GetHttpRequestDataAsync();

        if (httpReqData != null)
        {
            var newResponse = httpReqData.CreateResponse();
            await newResponse.WriteAsJsonAsync(new { FooStatus = "Bar" });

            // Update invocation result with the new http response.
             context.GetInvocationResult().Value = newResponse;
        }
    }
}

Refer the sample app for more middleware examples.

@davidpeden3
Copy link

context.GetInvocationResult().Value = newHttpResponse;

Can we get a set method instead? This API feels awkward.

context.SetInvocationResult(newHttpResponse); would be so much cleaner and more discoverable.

@velocitysystems
Copy link

@davidpeden3 Agreed. The method naming feels contrary to the intended use and purpose. Since this is a ‘set’ operation, something like SetInvocationResult or SetResponse feels more suitable and discoverable.

@kshyju
Copy link
Member

kshyju commented May 11, 2022

@kshyju following up on the documentation 🙂

@SeanFeldman The middleware section on the official documentation page has been updated with information about some of the new APIs we exposed along with an example middleware implementation.

We will continue to iterate. Thanks for the reminder.

@ghost ghost locked as resolved and limited conversation to collaborators Jun 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.