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

Handle unknown exception on authorization check #270

Merged
merged 3 commits into from
Mar 2, 2017

Conversation

rdhabalia
Copy link
Contributor

Motivation

Right now, topic/PartitionMetadata lookup gives 401(unauthorized) error-code instead of 500(internalServerError) even if request fails with unknown-exception (eg. zk_session is not available). It may lead incorrect error-handling at client side.

Modifications

Return 500 if request fails with unknown exception.

Result

Broker returns correct error-code on lookup failure.

@rdhabalia rdhabalia added the type/bug The PR fixed a bug or issue reported a bug label Mar 2, 2017
@rdhabalia rdhabalia added this to the 1.17 milestone Mar 2, 2017
@rdhabalia rdhabalia self-assigned this Mar 2, 2017
@merlimat
Copy link
Contributor

merlimat commented Mar 2, 2017

@rdhabalia It seems there are some genuine test failures :

Tests run: 527, Failures: 6, Errors: 0, Skipped: 0, Time elapsed: 1,062.603 sec <<< FAILURE! - in TestSuite
socketTest(com.yahoo.pulsar.websocket.proxy.ProxyPublishConsumeTest)  Time elapsed: 10.098 sec  <<< FAILURE!
org.testng.internal.thread.ThreadTimeoutException: Method org.testng.internal.TestNGMethod.socketTest() didn't finish within the time-out 10000
	at com.yahoo.pulsar.websocket.proxy.ProxyPublishConsumeTest.socketTest(ProxyPublishConsumeTest.java:102)
testInternalServerExceptionOnLookup(com.yahoo.pulsar.client.api.AuthenticatedProducerConsumerTest)  Time elapsed: 0.109 sec  <<< FAILURE!
java.lang.AssertionError
	at com.yahoo.pulsar.client.api.AuthenticatedProducerConsumerTest.testInternalServerExceptionOnLookup(AuthenticatedProducerConsumerTest.java:228)
testMultipleBrokerDifferentClusterLookup(com.yahoo.pulsar.client.api.BrokerServiceLookupTest)  Time elapsed: 0.215 sec  <<< FAILURE!
com.yahoo.pulsar.client.api.PulsarClientException$LookupException: com.yahoo.pulsar.client.api.PulsarClientException: Authorization failed null on cluster persistent://my-property2/use2/my-ns/my-topic1 with error null
testMultipleBrokerLookup(com.yahoo.pulsar.client.api.BrokerServiceLookupTest)  Time elapsed: 57.306 sec  <<< FAILURE!
com.yahoo.pulsar.client.api.PulsarClientException: java.util.concurrent.CompletionException: com.yahoo.pulsar.client.api.PulsarClientException$LookupException: java.lang.NullPointerException
Caused by: java.util.concurrent.CompletionException: com.yahoo.pulsar.client.api.PulsarClientException$LookupException: java.lang.NullPointerException
Caused by: com.yahoo.pulsar.client.api.PulsarClientException$LookupException: java.lang.NullPointerException
testPartitionTopicLookup(com.yahoo.pulsar.client.api.BrokerServiceLookupTest)  Time elapsed: 62.131 sec  <<< FAILURE!
com.yahoo.pulsar.client.api.PulsarClientException: java.util.concurrent.CompletionException: com.yahoo.pulsar.client.api.PulsarClientException$LookupException: java.lang.NullPointerException
Caused by: java.util.concurrent.CompletionException: com.yahoo.pulsar.client.api.PulsarClientException$LookupException: java.lang.NullPointerException
Caused by: com.yahoo.pulsar.client.api.PulsarClientException$LookupException: java.lang.NullPointerException
testWebserviceServiceTls(com.yahoo.pulsar.client.api.BrokerServiceLookupTest)  Time elapsed: 0.357 sec  <<< FAILURE!
javax.net.ssl.SSLException: Unrecognized SSL message, plaintext connection?
	at com.yahoo.pulsar.client.api.BrokerServiceLookupTest.testWebserviceServiceTls(BrokerServiceLookupTest.java:421)

@rdhabalia
Copy link
Contributor Author

yes, looking into it.

@merlimat merlimat merged commit 895509d into apache:master Mar 2, 2017
rdhabalia added a commit that referenced this pull request Mar 10, 2017
* Handle unknown exception on authorization check

* fix test:init AuthorizationManager on Auth-enable and set status-500 on topic-lookup-internal-failire

* fail partitionMetadata on zk-failure
@rdhabalia rdhabalia deleted the 401 branch June 21, 2017 18:54
hrsakai pushed a commit to hrsakai/pulsar that referenced this pull request Dec 10, 2020
* When CGO is enabled, use C version of ZStd

* Fixed formatting
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request May 23, 2024
* ManagedCursor: manually serialise PositionInfo
* Add tests and save last serialized side to prevent reallocations
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request May 28, 2024
* ManagedCursor: manually serialise PositionInfo
* Add tests and save last serialized side to prevent reallocations

(cherry picked from commit 8a365d0)
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Sep 9, 2024
* ManagedCursor: manually serialise PositionInfo
* Add tests and save last serialized side to prevent reallocations

(cherry picked from commit 8a365d0)
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Sep 16, 2024
* ManagedCursor: manually serialise PositionInfo
* Add tests and save last serialized side to prevent reallocations

(cherry picked from commit 8a365d0)
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Sep 24, 2024
* ManagedCursor: manually serialise PositionInfo
* Add tests and save last serialized side to prevent reallocations

(cherry picked from commit 8a365d0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants