fix: Return an error on publish if the publisher's state is not RUNNING

[tmdiep]

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes internal issue b/156525288.

[codecov]

Codecov Report

Merging #81 into master will increase coverage by 0.09%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##             master      #81      +/-   ##
============================================
+ Coverage     60.11%   60.21%   +0.09%     
  Complexity      388      388              
============================================
  Files            90       90              
  Lines          2046     2046              
  Branches        170      171       +1     
============================================
+ Hits           1230     1232       +2     
+ Misses          724      722       -2     
  Partials         92       92              

Impacted Files	Coverage Δ	Complexity Δ
.../cloud/pubsublite/internal/wire/PublisherImpl.java	78.15% <75.00%> (+1.68%)	20.00 <0.00> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb35590...5f57076. Read the comment docs.

[tmdiep]

Not sure if this is the best solution.
I didn't call onPermanentError() because the publisher may already be in a failed or terminated state.

[dpcollins-google]

Not sure if this is the best solution.
I didn't call onPermanentError() because the publisher may already be in a failed or terminated state.

This should be okay. ProxyService will handle this gracefully, please add onPermanentError.

[tmdiep]

I didn't call onPermanentError() because the publisher may already be in a failed or terminated state.

This should be okay. ProxyService will handle this gracefully, please add onPermanentError.

Done. PTAL at revised implementation in PublisherImpl.

[dpcollins-google]

Please remove this method. You can check the permanent error handlerr to determine if a permanent error occurred.

[tmdiep]

Yeah, that was the first thing I tried, but the listeners don't get called. I traced through and think it's because AbstractService::notifyFailed just throws an IllegalStateException when the state is New or Terminated, and doesn't transition to Failed. Since there's no state transition, the listeners aren't called.

I couldn't see another way to verify that PublisherImpl had permanently shutdown, so added isShutdown(). But could just omit that check in the test.

[dpcollins-google]

Put it into STARTING or STOPPING by making the startAsync or stopAsync method of the RetryingConnection block on a CountdownLatch. Then try to publish to trigger this case, and then let the method proceed.

[tmdiep]

Thanks, CountDownLatch is useful. I looked into your suggestion and noticed that all of our doStarts are synchronous. The only scenario that really triggers the original bug (b/156525288) is when the user publishes before calling startAsync, and so left the test to check NEW.

I removed the isShutdown() and just verified that the service has failed permanently by checking that an IllegalStateException is thrown if the user then tries to start the service. This occurs if they try to start PublisherImpl or any proxy such as WrappingPublisher.

[dpcollins-google]

See comments