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

middleware/errorhandler: Implement Hijacker so it works with ws proxy #1971

Merged
merged 2 commits into from
Nov 10, 2016

Conversation

ekimekim
Copy link
Contributor

@ekimekim ekimekim commented Nov 2, 2016

The lack of this was causing 500s when errorhandler-wrapped requests were hijacked, such as for websocket proxying

@rade
Copy link
Member

rade commented Nov 3, 2016

Some details please. What is the user-visible impact of the problem this is fixing, and since when has it been present?

@ekimekim
Copy link
Contributor Author

ekimekim commented Nov 3, 2016

No scope user impact - this middleware is only used by authfe in the service. In the service, this bug means that we can't use the admin scope instance, and has been in dev and prod since late last week. This only affects the admin endpoints, and scope is the only admin endpoint to use websockets.

@@ -78,3 +81,14 @@ func (i *errorInterceptor) Write(data []byte) (int, error) {
}
return len(data), nil
}

// errorInterceptor also implements net.Hijacker, to let the downstream Handler
// hijack the connection. This is needed by the app-mapper's proxy.

This comment was marked as abuse.

@2opremio
Copy link
Contributor

2opremio commented Nov 8, 2016

LGTM

@ekimekim ekimekim merged commit f9df493 into master Nov 10, 2016
@ekimekim ekimekim deleted the mike/errorhandler/implement-hijack branch November 10, 2016 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants