GoogleCloudPlatform / functions-framework-python

FaaS (Function as a service) framework for writing portable Python functions
https://pypi.org/p/functions-framework/
Apache License 2.0
872 stars 118 forks source link

But don't take away exception handling! #52

Closed jakebiesinger-onduo closed 4 years ago

jakebiesinger-onduo commented 4 years ago

Howdy!

I love the idea of this repository. We have tons of GCP Cloud Functions and Cloud Runs (what an awful noun). It can be hard for a dev to switch between the two, particularly to go to Cloud Run land when they've never seen a Dockerfile before.

But we're moving more and more of our stuff into Cloud Run because it offers better options for handling exceptions. Many of these are super-duper simple functions and Cloud Run feels like overkill, just to allow me to specify how we should handle those exceptions.

I realize this is more like a feature request for Cloud Functions, but consider how easy you could make this for everyone:

def entrypoint_do_thing(request):
  if request.args['foo'] == 'bar:
    return 'some_data', 200
  else:
    raise BadnessError()

@functions_framework.error_handler(BadnessError)
def handle_badness(e):
  return 'badness happened', 400

Something like the above would be simple and familiar for lots of folks since it looks like Flask error handling, but you still wouldn't have to expose the underlying Flask app to the world.

Thoughts?

di commented 4 years ago

Hi @jakebiesinger-onduo, thanks for the feature request. I think it's unlikely that we'd just implement this for the Python framework -- this pattern could be useful for other languages as well.

Could you open this at https://github.com/GoogleCloudPlatform/functions-framework as a feature request for the overall contract instead?

jakebiesinger-onduo commented 4 years ago

Sorry for the noise. I somehow missed this was the transfer's destination issue instead of the source issue.

Edit-- oh! Github just 302's us to the same issue once a transfer happens. How confusing.

di commented 4 years ago

Ah, sorry about the confusion @jakebiesinger-onduo. I was hoping that you'd be able to make a more generic request for error handling than this very Python-specific request, but it's probably fine as-is, we can figure it out.

grant commented 4 years ago

Hey @jakebiesinger-onduo, Glad you like the idea of the project 😄 . I transferred the issue/feature request as suggested above. (Should have commented I did this as well).

I created a PR that better describes the ideal error handling for all Function Frameworks.


I'm not sure if there's specific features we should add to the frameworks for better error handling as suggested. A decorator for functions works in some languages.

Do you think a helper method could do something similar in python? Can you describe the actual and expected behavior? I'm not sure of the specific suggestion or improvement we're trying to make within the Functions Framework rather than the user's code.

grant commented 4 years ago

Hi @jakebiesinger-onduo,

I believe you can do the same thing with possibly wrapping your function in an app with a router. That's what we can with Express. https://medium.com/google-cloud/express-routing-with-google-cloud-functions-36fb55885c68

Not sure what to change here in the Functions Framework. Any thoughts? I was thinking of closing this issue.

di commented 4 years ago

I had suggested something similar when Jake asked about this in the beta-testers group:

from flask import redirect

class SomeException(Exception):
    pass

class ErrorFunction:
    def __init__(self, f):
        self.f = f
        self.errorhandlers = {}

    def __call__(self, request):
        try:
            return self.f(request)
        except Exception as e:
            handler = self.errorhandlers.get(type(e), self.default)
            return handler(e)

    def default(self, e):
        raise e

    def errorhandler(self, exception):
        def wrapper(handler):
            self.errorhandlers[exception] = handler

        return wrapper

@ErrorFunction
def test(request):
    raise SomeException

@test.errorhandler(SomeException)
def handle_some_exception(e):
    return redirect("http://zombo.com")

but this is tedious for users to do again and again, and (if I understand correctly) he was hoping for a more standard way to do do this across all Functions Frameworks.

jakebiesinger-onduo commented 4 years ago

Bingo, @di. It's not that I can't do this myself, it's that it's onerous across dozens of functions (especially given how hard private shared libraries already are in Cloud Functions!).

Error handling feels like it should be a first-class citizen of a framework-- you're forcing me to implement my own handler framework instead of just giving me hooks that operate in a consistent, well-documented way.

Across dozens of functions in different GH repos + GCP projects, there's lots of pain associated with homebrewing a handler framework and lots of dividends in having sane, uniform hooks.

jakebiesinger-onduo commented 4 years ago

As for how this would work, adopting @di's example, I'd hope to be able to substitute the custom boilerplate with officially supported hooks:

from functions_framework import error_handler

def cf_entrypoint(request):
  if not request.args['foo']:
    raise SomeException('missing foo!')

@error_handler(SomeException)
def handle_some_exception(e):
    return redirect("http://zombo.com")

As shown, it'd be nice to not have to decorate the entrypoint in any way.

grant commented 4 years ago

Happy to listen to generic proposals. I don't think every language has this issue, many languages can add error handling in an idiomatic way.

I think it would be fine for the Python Functions Framework to have a custom decorator pattern if that is customary in that language and isn't a pain to support/add.

Also happy to transfer this issue back to the python repo if requested.

ChristianSauer commented 4 years ago

In my opion, this is absolutely a must - there are dozens of possible errors in any non trivial python cloud function (wrong HTTP method, maybe cannot deserialize json body, various validation errors etc.) which totally bloat the cloud function. Additionally, the cloud function often should adhere to some default api errors, e.g. RFC 7807 errors - so we need a single exit which can format all errors appropriately.

The idiomatic way to solve this in Flask (the underlying webframework used py cloud-functions) is to use a global exception handler (https://flask.palletsprojects.com/en/1.1.x/errorhandling/) . But due to the design of the framework, it is impossible to use it directly in the cloud function - I would recommend surfacing it through the framework, ideally some sort of decorator. I raised this issue independently here: https://github.com/GoogleCloudPlatform/functions-framework-python/issues/48

Here is a small screenshot which shows the error handling section of my cloud function: image

jakebiesinger-onduo commented 4 years ago

@grant if this is not desired as a generic functions-framework feature, perhaps we should migrate this issue back to GoogleCloudPlatform/functions-framework-python?

grant commented 4 years ago

@grant if this is not desired as a generic functions-framework feature, perhaps we should migrate this issue back to GoogleCloudPlatform/functions-framework-python?

Transferred.