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

Clarification on error handling #152

Open
dippynark opened this issue Apr 26, 2021 · 5 comments
Open

Clarification on error handling #152

dippynark opened this issue Apr 26, 2021 · 5 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@dippynark
Copy link

The example auth module dispatches a HTTP request to determine whether to allow the request: https://github.com/tetratelabs/proxy-wasm-go-sdk/blob/main/examples/http_auth_random/main.go#L61-L65

This logs a critical error and returns a continue action. Does this mean that if the auth service is down all requests will be allowed through? i.e. does this code fail open?

If so, how would you fail closed? You could use something like proxywasm.SendHttpResponse(403... but then how do you handle any error returned by that function?

@mathetake
Copy link
Member

mathetake commented Apr 26, 2021

does this code fail open?

hi @dippynark, that's a good question. actually it does fail open.

As of now, Go SDK doesn't support host calls to close streams in case of failure (named closeUpstream and closeDownStream) yet. But it's easy to implement so will do in a week or so. Thanks!

@mathetake mathetake added enhancement New feature or request question Further information is requested labels Apr 26, 2021
@dippynark
Copy link
Author

Awesome, thanks! I guess will leave this open until then? Happy to close it though as it answers the question

@mathetake
Copy link
Member

yeah let's leave this open until then. Thanks!

@skirsten
Copy link

Hi,

I did not find any api in the spec that would support closing the connection for HTTP context.
The referenced closeUpstream and closeDownStream are supposedly only for TCP context.

You could use something like proxywasm.SendHttpResponse(403... but then how do you handle any error returned by that function?

This is exactly what I need to know. Could we use panic in that case? I need to somehow make sure the request processing is aborted.

@skirsten
Copy link

Looking at the rust examples https://github.com/proxy-wasm/proxy-wasm-rust-sdk/blob/master/examples/http_auth_random.rs and https://github.com/proxy-wasm/proxy-wasm-rust-sdk/blob/master/examples/http_headers.rs it seems they just leave the processing paused or never resume it in the case there could be an error.
Does this mean that the request will forever stay open or is it smart enough to notice there are no remaining callbacks and abort the request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants