{"action":"created","comment":{"url":"https://api.github.com/repos/netty/netty/pulls/comments/31387812","id":31387812,"diff_hunk":"@@ -94,62 +84,87 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t\n         }\n     }\n \n-    private boolean initPipeline(ChannelHandlerContext ctx) {\n+    private boolean configurePipeline(ChannelHandlerContext ctx) {\n         // Get the SslHandler from the ChannelPipeline so we can obtain the\n         // SslEngine from it.\n         SslHandler handler = ctx.pipeline().get(SslHandler.class);\n         if (handler == null) {\n-            // HTTP2 is negotiated through SSL.\n-            throw new IllegalStateException(\"SslHandler is needed for HTTP2\");\n+            // HTTP/2 is negotiated through SSL.\n+            throw new IllegalStateException(\"cannot find a SslHandler in the pipeline (required for HTTP/2)\");\n+        }\n+\n+        if (!handler.handshakeFuture().isDone()) {\n+            return false;\n+        }\n+\n+        SelectedProtocol protocol;\n+        try {\n+            protocol = selectProtocol(handler);\n+        } catch (Exception e) {\n+            throw new IllegalStateException(\"failed to get the selected protocol\", e);\n+        }\n+\n+        if (protocol == null) {\n+            throw new IllegalStateException(\"unknown protocol\");\n         }\n \n-        SelectedProtocol protocol = getProtocol(handler.engine());\n         switch (protocol) {\n-            case UNKNOWN:\n-                // Not done with choosing the protocol, so just return here for now,\n-                return false;\n             case HTTP_2:\n-                addHttp2Handlers(ctx);\n+                try {\n+                    configureHttp2(ctx);\n+                } catch (Exception e) {\n+                    throw new IllegalStateException(\"failed to configure a HTTP/2 pipeline\", e);\n+                }\n                 break;\n             case HTTP_1_0:\n             case HTTP_1_1:\n-                addHttpHandlers(ctx);\n+                try {\n+                    configureHttp1(ctx);\n+                } catch (Exception e) {\n+                    throw new IllegalStateException(\"failed to configure a HTTP/1 pipeline\", e);\n+                }\n                 break;\n-            default:","path":"codec-http2/src/main/java/io/netty/handler/codec/http2/Http2OrHttpChooser.java","position":142,"original_position":142,"commit_id":"8a1f47b0f8cdca9e54274b32c842c8ee74e54240","original_commit_id":"8a1f47b0f8cdca9e54274b32c842c8ee74e54240","user":{"login":"normanmaurer","id":439362,"avatar_url":"https://avatars.githubusercontent.com/u/439362?v=3","gravatar_id":"","url":"https://api.github.com/users/normanmaurer","html_url":"https://github.com/normanmaurer","followers_url":"https://api.github.com/users/normanmaurer/followers","following_url":"https://api.github.com/users/normanmaurer/following{/other_user}","gists_url":"https://api.github.com/users/normanmaurer/gists{/gist_id}","starred_url":"https://api.github.com/users/normanmaurer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/normanmaurer/subscriptions","organizations_url":"https://api.github.com/users/normanmaurer/orgs","repos_url":"https://api.github.com/users/normanmaurer/repos","events_url":"https://api.github.com/users/normanmaurer/events{/privacy}","received_events_url":"https://api.github.com/users/normanmaurer/received_events","type":"User","site_admin":false},"body":"same comment as above","created_at":"2015-05-31T07:43:22Z","updated_at":"2015-05-31T07:43:22Z","html_url":"https://github.com/netty/netty/pull/3846#discussion_r31387812","pull_request_url":"https://api.github.com/repos/netty/netty/pulls/3846","_links":{"self":{"href":"https://api.github.com/repos/netty/netty/pulls/comments/31387812"},"html":{"href":"https://github.com/netty/netty/pull/3846#discussion_r31387812"},"pull_request":{"href":"https://api.github.com/repos/netty/netty/pulls/3846"}}},"pull_request":{"url":"https://api.github.com/repos/netty/netty/pulls/3846","id":36562641,"html_url":"https://github.com/netty/netty/pull/3846","diff_url":"https://github.com/netty/netty/pull/3846.diff","patch_url":"https://github.com/netty/netty/pull/3846.patch","issue_url":"https://api.github.com/repos/netty/netty/issues/3846","number":3846,"state":"open","locked":false,"title":"Improve the API design of Http2OrHttpChooser and SpdyOrHttpChooser","user":{"login":"trustin","id":173918,"avatar_url":"https://avatars.githubusercontent.com/u/173918?v=3","gravatar_id":"","url":"https://api.github.com/users/trustin","html_url":"https://github.com/trustin","followers_url":"https://api.github.com/users/trustin/followers","following_url":"https://api.github.com/users/trustin/following{/other_user}","gists_url":"https://api.github.com/users/trustin/gists{/gist_id}","starred_url":"https://api.github.com/users/trustin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trustin/subscriptions","organizations_url":"https://api.github.com/users/trustin/orgs","repos_url":"https://api.github.com/users/trustin/repos","events_url":"https://api.github.com/users/trustin/events{/privacy}","received_events_url":"https://api.github.com/users/trustin/received_events","type":"User","site_admin":false},"body":"Motivation:\r\n\r\nWhen setting up an HTTP/1 or HTTP/2 (or SPDY) pipeline, a user usually ends up with adding arbitrary set of handlers.\r\n\r\nHttp2OrHttpChooser and SpdyOrHttpChooser have two abstract methods (create\\*Handler()) that expect a user to return a single handler, and also have add\\*Handlers() methods that add the handler returned by create\\*Handler() to the pipeline as well as the pre-defined set of handlers.\r\n\r\nThe problem is, some users (read: I) don't need all of them or the user wants to add more than one handler. For example, take a look at io.netty.example.http2.tiles.Http2OrHttpHandler, which works around this issue by overriding addHttp2Handlers() and making createHttp2RequestHandler() a no-op.\r\n\r\nModifications:\r\n\r\n- Replace add\\*Handlers() and create\\*Handler() with configure\\*()\r\n- Rename getProtocol() to selectProtocol() to make what it does clear\r\n- Provide the default implementation of selectProtocol()\r\n- Remove SelectedProtocol.UNKNOWN and use null instead, because 'UNKNOWN' is not a protocol\r\n- Proper exception handling in the \\*OrHttpChooser so that the exception is logged and the connection is closed when failed to select a protocol\r\n- Make SpdyClient example always use SSL. It was always using SSL anyway.\r\n- Implement SslHandshakeCompletionEvent.toString() for debuggability\r\n- Remove an orphaned class: JettyNpnSslSession\r\n- Add SslHandler.applicationProtocol() to get the name of the application protocol\r\n  - SSLSession.getProtocol() now returns transport-layer protocol name only, so that it conforms to its contract.\r\n\r\nResult:\r\n\r\n- \\*OrHttpChooser have better API.\r\n- \\*OrHttpChooser handle protocol selection failure properly.\r\n- SSLSession.getProtocol() now conforms to its contract.\r\n- SpdyClient example works with SpdyServer example out of the box","created_at":"2015-05-30T08:55:10Z","updated_at":"2015-05-31T07:43:22Z","closed_at":null,"merged_at":null,"merge_commit_sha":"54620ab2e2a8bdd1375a7346132277a5b4b469aa","assignee":null,"milestone":{"url":"https://api.github.com/repos/netty/netty/milestones/117","html_url":"https://github.com/netty/netty/milestones/4.1.0.Beta6","labels_url":"https://api.github.com/repos/netty/netty/milestones/117/labels","id":1100817,"number":117,"title":"4.1.0.Beta6","description":"","creator":{"login":"normanmaurer","id":439362,"avatar_url":"https://avatars.githubusercontent.com/u/439362?v=3","gravatar_id":"","url":"https://api.github.com/users/normanmaurer","html_url":"https://github.com/normanmaurer","followers_url":"https://api.github.com/users/normanmaurer/followers","following_url":"https://api.github.com/users/normanmaurer/following{/other_user}","gists_url":"https://api.github.com/users/normanmaurer/gists{/gist_id}","starred_url":"https://api.github.com/users/normanmaurer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/normanmaurer/subscriptions","organizations_url":"https://api.github.com/users/normanmaurer/orgs","repos_url":"https://api.github.com/users/normanmaurer/repos","events_url":"https://api.github.com/users/normanmaurer/events{/privacy}","received_events_url":"https://api.github.com/users/normanmaurer/received_events","type":"User","site_admin":false},"open_issues":68,"closed_issues":14,"state":"open","created_at":"2015-05-07T19:29:09Z","updated_at":"2015-05-30T08:55:21Z","due_on":null,"closed_at":null},"commits_url":"https://api.github.com/repos/netty/netty/pulls/3846/commits","review_comments_url":"https://api.github.com/repos/netty/netty/pulls/3846/comments","review_comment_url":"https://api.github.com/repos/netty/netty/pulls/comments{/number}","comments_url":"https://api.github.com/repos/netty/netty/issues/3846/comments","statuses_url":"https://api.github.com/repos/netty/netty/statuses/8a1f47b0f8cdca9e54274b32c842c8ee74e54240","head":{"label":"netty:http2_or_http_chooser_api","ref":"http2_or_http_chooser_api","sha":"8a1f47b0f8cdca9e54274b32c842c8ee74e54240","user":{"login":"netty","id":473791,"avatar_url":"https://avatars.githubusercontent.com/u/473791?v=3","gravatar_id":"","url":"https://api.github.com/users/netty","html_url":"https://github.com/netty","followers_url":"https://api.github.com/users/netty/followers","following_url":"https://api.github.com/users/netty/following{/other_user}","gists_url":"https://api.github.com/users/netty/gists{/gist_id}","starred_url":"https://api.github.com/users/netty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netty/subscriptions","organizations_url":"https://api.github.com/users/netty/orgs","repos_url":"https://api.github.com/users/netty/repos","events_url":"https://api.github.com/users/netty/events{/privacy}","received_events_url":"https://api.github.com/users/netty/received_events","type":"Organization","site_admin":false},"repo":{"id":1064563,"name":"netty","full_name":"netty/netty","owner":{"login":"netty","id":473791,"avatar_url":"https://avatars.githubusercontent.com/u/473791?v=3","gravatar_id":"","url":"https://api.github.com/users/netty","html_url":"https://github.com/netty","followers_url":"https://api.github.com/users/netty/followers","following_url":"https://api.github.com/users/netty/following{/other_user}","gists_url":"https://api.github.com/users/netty/gists{/gist_id}","starred_url":"https://api.github.com/users/netty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netty/subscriptions","organizations_url":"https://api.github.com/users/netty/orgs","repos_url":"https://api.github.com/users/netty/repos","events_url":"https://api.github.com/users/netty/events{/privacy}","received_events_url":"https://api.github.com/users/netty/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/netty/netty","description":"Netty project - an event-driven asynchronous network application framework","fork":false,"url":"https://api.github.com/repos/netty/netty","forks_url":"https://api.github.com/repos/netty/netty/forks","keys_url":"https://api.github.com/repos/netty/netty/keys{/key_id}","collaborators_url":"https://api.github.com/repos/netty/netty/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/netty/netty/teams","hooks_url":"https://api.github.com/repos/netty/netty/hooks","issue_events_url":"https://api.github.com/repos/netty/netty/issues/events{/number}","events_url":"https://api.github.com/repos/netty/netty/events","assignees_url":"https://api.github.com/repos/netty/netty/assignees{/user}","branches_url":"https://api.github.com/repos/netty/netty/branches{/branch}","tags_url":"https://api.github.com/repos/netty/netty/tags","blobs_url":"https://api.github.com/repos/netty/netty/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/netty/netty/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/netty/netty/git/refs{/sha}","trees_url":"https://api.github.com/repos/netty/netty/git/trees{/sha}","statuses_url":"https://api.github.com/repos/netty/netty/statuses/{sha}","languages_url":"https://api.github.com/repos/netty/netty/languages","stargazers_url":"https://api.github.com/repos/netty/netty/stargazers","contributors_url":"https://api.github.com/repos/netty/netty/contributors","subscribers_url":"https://api.github.com/repos/netty/netty/subscribers","subscription_url":"https://api.github.com/repos/netty/netty/subscription","commits_url":"https://api.github.com/repos/netty/netty/commits{/sha}","git_commits_url":"https://api.github.com/repos/netty/netty/git/commits{/sha}","comments_url":"https://api.github.com/repos/netty/netty/comments{/number}","issue_comment_url":"https://api.github.com/repos/netty/netty/issues/comments{/number}","contents_url":"https://api.github.com/repos/netty/netty/contents/{+path}","compare_url":"https://api.github.com/repos/netty/netty/compare/{base}...{head}","merges_url":"https://api.github.com/repos/netty/netty/merges","archive_url":"https://api.github.com/repos/netty/netty/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/netty/netty/downloads","issues_url":"https://api.github.com/repos/netty/netty/issues{/number}","pulls_url":"https://api.github.com/repos/netty/netty/pulls{/number}","milestones_url":"https://api.github.com/repos/netty/netty/milestones{/number}","notifications_url":"https://api.github.com/repos/netty/netty/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/netty/netty/labels{/name}","releases_url":"https://api.github.com/repos/netty/netty/releases{/id}","created_at":"2010-11-09T09:22:21Z","updated_at":"2015-05-30T16:54:50Z","pushed_at":"2015-05-31T07:36:32Z","git_url":"git://github.com/netty/netty.git","ssh_url":"git@github.com:netty/netty.git","clone_url":"https://github.com/netty/netty.git","svn_url":"https://github.com/netty/netty","homepage":"http://netty.io/","size":130803,"stargazers_count":4693,"watchers_count":4693,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2312,"mirror_url":null,"open_issues_count":281,"forks":2312,"open_issues":281,"watchers":4693,"default_branch":"master"}},"base":{"label":"netty:4.1","ref":"4.1","sha":"2a848ce1c94100e9fd3e063740d66187f17a3e8c","user":{"login":"netty","id":473791,"avatar_url":"https://avatars.githubusercontent.com/u/473791?v=3","gravatar_id":"","url":"https://api.github.com/users/netty","html_url":"https://github.com/netty","followers_url":"https://api.github.com/users/netty/followers","following_url":"https://api.github.com/users/netty/following{/other_user}","gists_url":"https://api.github.com/users/netty/gists{/gist_id}","starred_url":"https://api.github.com/users/netty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netty/subscriptions","organizations_url":"https://api.github.com/users/netty/orgs","repos_url":"https://api.github.com/users/netty/repos","events_url":"https://api.github.com/users/netty/events{/privacy}","received_events_url":"https://api.github.com/users/netty/received_events","type":"Organization","site_admin":false},"repo":{"id":1064563,"name":"netty","full_name":"netty/netty","owner":{"login":"netty","id":473791,"avatar_url":"https://avatars.githubusercontent.com/u/473791?v=3","gravatar_id":"","url":"https://api.github.com/users/netty","html_url":"https://github.com/netty","followers_url":"https://api.github.com/users/netty/followers","following_url":"https://api.github.com/users/netty/following{/other_user}","gists_url":"https://api.github.com/users/netty/gists{/gist_id}","starred_url":"https://api.github.com/users/netty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netty/subscriptions","organizations_url":"https://api.github.com/users/netty/orgs","repos_url":"https://api.github.com/users/netty/repos","events_url":"https://api.github.com/users/netty/events{/privacy}","received_events_url":"https://api.github.com/users/netty/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/netty/netty","description":"Netty project - an event-driven asynchronous network application framework","fork":false,"url":"https://api.github.com/repos/netty/netty","forks_url":"https://api.github.com/repos/netty/netty/forks","keys_url":"https://api.github.com/repos/netty/netty/keys{/key_id}","collaborators_url":"https://api.github.com/repos/netty/netty/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/netty/netty/teams","hooks_url":"https://api.github.com/repos/netty/netty/hooks","issue_events_url":"https://api.github.com/repos/netty/netty/issues/events{/number}","events_url":"https://api.github.com/repos/netty/netty/events","assignees_url":"https://api.github.com/repos/netty/netty/assignees{/user}","branches_url":"https://api.github.com/repos/netty/netty/branches{/branch}","tags_url":"https://api.github.com/repos/netty/netty/tags","blobs_url":"https://api.github.com/repos/netty/netty/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/netty/netty/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/netty/netty/git/refs{/sha}","trees_url":"https://api.github.com/repos/netty/netty/git/trees{/sha}","statuses_url":"https://api.github.com/repos/netty/netty/statuses/{sha}","languages_url":"https://api.github.com/repos/netty/netty/languages","stargazers_url":"https://api.github.com/repos/netty/netty/stargazers","contributors_url":"https://api.github.com/repos/netty/netty/contributors","subscribers_url":"https://api.github.com/repos/netty/netty/subscribers","subscription_url":"https://api.github.com/repos/netty/netty/subscription","commits_url":"https://api.github.com/repos/netty/netty/commits{/sha}","git_commits_url":"https://api.github.com/repos/netty/netty/git/commits{/sha}","comments_url":"https://api.github.com/repos/netty/netty/comments{/number}","issue_comment_url":"https://api.github.com/repos/netty/netty/issues/comments{/number}","contents_url":"https://api.github.com/repos/netty/netty/contents/{+path}","compare_url":"https://api.github.com/repos/netty/netty/compare/{base}...{head}","merges_url":"https://api.github.com/repos/netty/netty/merges","archive_url":"https://api.github.com/repos/netty/netty/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/netty/netty/downloads","issues_url":"https://api.github.com/repos/netty/netty/issues{/number}","pulls_url":"https://api.github.com/repos/netty/netty/pulls{/number}","milestones_url":"https://api.github.com/repos/netty/netty/milestones{/number}","notifications_url":"https://api.github.com/repos/netty/netty/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/netty/netty/labels{/name}","releases_url":"https://api.github.com/repos/netty/netty/releases{/id}","created_at":"2010-11-09T09:22:21Z","updated_at":"2015-05-30T16:54:50Z","pushed_at":"2015-05-31T07:36:32Z","git_url":"git://github.com/netty/netty.git","ssh_url":"git@github.com:netty/netty.git","clone_url":"https://github.com/netty/netty.git","svn_url":"https://github.com/netty/netty","homepage":"http://netty.io/","size":130803,"stargazers_count":4693,"watchers_count":4693,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2312,"mirror_url":null,"open_issues_count":281,"forks":2312,"open_issues":281,"watchers":4693,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/netty/netty/pulls/3846"},"html":{"href":"https://github.com/netty/netty/pull/3846"},"issue":{"href":"https://api.github.com/repos/netty/netty/issues/3846"},"comments":{"href":"https://api.github.com/repos/netty/netty/issues/3846/comments"},"review_comments":{"href":"https://api.github.com/repos/netty/netty/pulls/3846/comments"},"review_comment":{"href":"https://api.github.com/repos/netty/netty/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/netty/netty/pulls/3846/commits"},"statuses":{"href":"https://api.github.com/repos/netty/netty/statuses/8a1f47b0f8cdca9e54274b32c842c8ee74e54240"}}}}