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
Support pluggable error logging in shelf_io #2
Comments
This already exists: if it's used within an error Zone, |
@kevmoo I wonder if this is something we to try to tackle quickly in case it's easier to do with a breaking change. Do you have any thoughts on what the best API would be? The error zone behavior we have now helps with exceptions other than Lines 90 to 104 in 5a77d25
|
I just did this here: https://github.com/GoogleCloudPlatform/functions-framework-dart/blob/main/functions_framework/lib/src/logging.dart Not sure what else we'd do... Are you thinking we do this in |
My goal is to be able to use |
Oh! That would be nice – just for testing! Agreed here!
…On Fri, Dec 4, 2020 at 2:50 PM Nate Bosch ***@***.***> wrote:
My goal is to be able to use shelf_io without risk of writing to stderr.
I don't think that is feasible today because if a request comes in that
can't be parsed we hit the code path I linked above which will do a
stderr.writeln.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEFCW7GTHGDKPA4367QTLSTFRSJANCNFSM4A47KE5Q>
.
|
Could we just have an optional function param all the way down for |
It looks feasible. I almost wonder if we should drop the behavior around error zones entirely though. That makes it breaking but it means there aren't two ways to handle the same problem. WDYT? |
@jakemac53 @kevmoo and I chatted about this. Our biggest concern was that the removal of hidden magic behavior, is also hidden. We don't like that there would be a breaking change with no static indication that anything was breaking. We don't expect all users carefully read the CHANGELOG for breaking changes. If we make the new onError argument required though, then it is statically breaking and will call attention during the migration. It's also sensible that the error handling be explicit at the call site - if the caller does want the behavior of writing to |
@kevmoo - I think at some point you had mentioned wanting to have access to the The downside is that we want to keep a nice static type for the argument, so we can't allow functions that don't want the If there are errors parsing the IO request we also wont' have a shelf request to give to the error handler, so the type would need to be There are no existing use cases for reading the |
Wow...this bug has been around for a while! Let's chat tomorrow. |
Fix Strong Mode error with latest shelf
This looks like it was accidentally closed to me, re-opening for now but feel free to close it again if I am wrong. |
No description provided.
The text was updated successfully, but these errors were encountered: