[Java] Intermittent java.util.NoSuchElementException when listing all paged items.

[pomortaz]

When listing all items as below, the for loop for (CertificateItem item : listResult) will throw a NoSuchElementException intermittently.

PagedList<CertificateItem> listResult = keyVaultClient.getCertificates(getVaultUri(), PAGELIST_MAX_CERTS).getBody();

for (CertificateItem item : listResult) {
    CertificateIdentifier id = new CertificateIdentifier(item.id());
}

The exception that is thrown and the stack trace:

java.util.NoSuchElementException
    at java.util.ArrayList$Itr.next(Unknown Source)
    at com.microsoft.azure.PagedList$ListItr.next(PagedList.java:150)
    at com.microsoft.azure.keyvault.test.CertificateOperationsTest.listCertificates(CertificateOperationsTest.java:905)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

The exception is thrown because there is no other elements left to get the next(). There should be a condition before .next() to check whether there is an element: Iterator.hasNext()

[jianghaolu]

I can't reproduce this. The only possible scenario I can think of that can cause this is a listNext() call that returns an empty list.

[pomortaz]

Yes, the client should be resilient to empty list, returned by service instead of throwing exception..

[jianghaolu]

I think the previous call should not give a next link in this scenario.

[pomortaz]

@jianghaolu any update on this issue?

[jianghaolu]

I meant this in my previous comment:

1st page : [1, 2, 3]
2nd page: [4, 5, 6]
3rd page: [] <----------- This is not a scenario we support.

[pomortaz]

Why non? The library should be resilient to empty page and iterate as long as the next page exists.

[jianghaolu]

Why would you return an empty page in the first place?

[pomortaz]

That depends on paging implementations on the service side. However, the good design on the client side is to be resilient with these issues instead of assuming that service is always returning the expected value.

[jianghaolu]

In my opinion, it's just the code on the server side not handling the edge condition well. You may think it's just an extra loop on your server side, on the client-side it's a round-trip REST call that brings no value to the client.

[pomortaz]

Fair point. In this case, service would return empty page with next page link. This doesn't mean that the client should fail hard because it didn't expect service to return empty page. The protocol design is that the page includes at most the page maximum, but there is no protocol on the minimum list in this case. Azure Java SDK is the only language that is failing on this.

[jianghaolu]

Azure Java SDK is the only language that is failing on this.

Yeah of course. No other language is helping you group all pages together.
Customers can't care less about protocols. They care about experience.

[jianghaolu]

With that being said, I agree it's a nice thing to have on the client-side. We already have an issue discussing paging improvements here: Azure/azure-sdk-for-java#1022. Feel free to jump in.

[tjprescott]

Azure Java SDK is the only language that is failing on this.

This fails on Python too. It throws a 'NoneType is not iterable' TypeError.

[jianghaolu]

@tjprescott
This is already fixed in master branch in Java SDK. In the upcoming beta3 release next week this will be available as a maven package.

This might affect every language that has a automatic paging feature. That includes Java, Python, and Ruby.

@annatisch @vishrutshah @veronicagg
When service returns an empty page, the automatic paging iterator in Java still returns true for method hasNext() based on the knowledge that there's one more page coming. We fixed that in Java by caching the next non-empty page. This is most easily reproduced in the list() methods in key vault management library.

[veronicagg]

For Ruby, we see this issue surface in datalake data plane services, we haven't generated these ones yet, we'll take a look into the issue soon.

[vishrutshah]

Ruby:
Verified in the key vaults list by resource group call at list_by_resource_group and it is NOT breaking in the case when server send next link where results is an empty array. Ruby is covered here. So removing ruby label.

[martinsawicki]

closing as it appears to have been addressed