[🐛 Bug]: BiDi events are not fired in order

[joerg1985]

What happened?

This is basically the same problem for BiDi as for CDP, see issue https://github.com/SeleniumHQ/selenium/issues/13845

I stumbled into this while implementing network interception, using the org.openqa.selenium.bidi.module.Network class. The events raised by the bidi connection are processed concurrently and not raised in order. This will make it hard to use things like org.openqa.selenium.bidi.module.Network as there is no guarantee network.responseStarted is the first event raised for this request.

So i think it might be worth looking into this and find a solution, before BiDi does leave the beta stage.

How can we reproduce the issue?

This is not very likley to be seen in the wild, as network responses take some time, but especially the `network.fetchError` might be raised a very short time after `network.responseStarted` e.g. in case of CORS errors.

Relevant log output

N/A

Operating System

Win 10 x64

Selenium version

4.23.0

What are the browser(s) and version(s) where you see this issue?

N/A

What are the browser driver(s) and version(s) where you see this issue?

N/A

Are you using Selenium Grid?

N/A

[github-actions[bot]]

@joerg1985, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[pujagani]

Thank you for raising this. Do you have an example/test that can help reproduce this?

[joerg1985]

@pujagani i can only provide some code to show this without a browser, see below.

As soon as the delay between sending network.beforeRequestSent and network.responseStarted is high enought everything works as expected. As soon as there are alot of events raised they get out of order, i guess at the point when one thread is not able to consume them sequentially.

I think it should be possible to have this in a real browser as described in the issue.

import java.io.UncheckedIOException;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.bidi.BiDi;
import org.openqa.selenium.bidi.HasBiDi;
import org.openqa.selenium.bidi.module.Network;
import org.openqa.selenium.remote.http.HttpClient;
import org.openqa.selenium.remote.http.HttpRequest;
import org.openqa.selenium.remote.http.HttpResponse;
import org.openqa.selenium.remote.http.Message;
import org.openqa.selenium.remote.http.TextMessage;
import org.openqa.selenium.remote.http.WebSocket;

public class Connection {

    private static WebSocket.Listener LISTENER;

    public static void main(String[] args) throws InterruptedException {
        int delay = 1; // << increase this to e.g. 40ms to see the code is running fine in case there is a delay between the events
        
        HttpClient client = new HttpClient() {
            @Override
            public WebSocket openSocket(HttpRequest request, WebSocket.Listener listener) {
                LISTENER = listener;
                AtomicLong state = new AtomicLong();

                return new WebSocket() {

                    @Override
                    public WebSocket send(Message message) {
                        long id = state.incrementAndGet();
                        
                        if (id < 4) {
                            // answer the register calls
                            LISTENER.accept(new TextMessage(
                                    "{\"id\": " + id + ", \"result\": {}, \"type\": \"success\"}"
                            ));
                        }  
                        
                        return this;
                    }

                    @Override
                    public void close() {

                    }

                };
            }

            @Override
            public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
                throw new UnsupportedOperationException("Not supported yet.");
            }
        };

        org.openqa.selenium.bidi.Connection connection = new org.openqa.selenium.bidi.Connection(client, "N/A");

        ConcurrentHashMap<String, Boolean> requests = new ConcurrentHashMap<>();

        Network network = new Network(new MockDriver(new BiDi(connection)));


        network.onBeforeRequestSent((before) -> {
            requests.put(before.getRequest().getRequestId(), Boolean.TRUE);
        });

        network.onResponseStarted((started) -> {
            if (requests.putIfAbsent(started.getRequest().getRequestId(), Boolean.FALSE) == null) {
                System.err.println(started.getRequest().getRequestId() + " NOT found in map");
                System.exit(666);
            }
        });

        // wait to ensure all listeners are in place
        Thread.sleep(2000);

        for (long i = 4; i < Long.MAX_VALUE; i++) {
            if (i % 100 == 0) {
                System.out.println("iteration: " + i);
            }
            
            LISTENER.accept(new TextMessage("{\"method\": \"network.beforeRequestSent\", \"params\": {"
                    + "\"context\": null,"
                    + "\"isBlocked\": true,"
                    + "\"navigation\": null,"
                    + "\"redirectCount\": 0,"
                    + "\"request\": {"
                    + "\"request\": \"" + i + "\","
                    + "\"url\": \"https://url\","
                    + "\"method\": \"GET\","
                    + "\"headers\": [],"
                    + "\"cookies\": [],"
                    + "\"headersSize\": 0,"
                    + "\"bodySize\": null"
                    + "},"
                    + "\"timestamp\": " + System.currentTimeMillis()+ ", "
                    + "\"initiator\": {\"type\" :\"other\"}"
                    + "}}"));
            
            if (delay > 0)
                Thread.sleep(delay);
            
            LISTENER.accept(new TextMessage("{\"method\": \"network.responseStarted\", \"params\": {"
                    + "\"context\": null,"
                    + "\"isBlocked\": true,"
                    + "\"navigation\": null,"
                    + "\"redirectCount\": 0,"
                    + "\"request\": {"
                    + "\"request\": \"" + i + "\","
                    + "\"url\": \"https://url\","
                    + "\"method\": \"GET\","
                    + "\"headers\": [],"
                    + "\"cookies\": [],"
                    + "\"headersSize\": 0,"
                    + "\"bodySize\": null"
                    + "},"
                    + "\"timestamp\": " + System.currentTimeMillis() + ", "
                    + "\"response\": {}"
                    + "}}"));
        }

    }

    private static class MockDriver implements WebDriver, HasBiDi {

        private final BiDi _bidi;

        public MockDriver(BiDi bidi) {
            _bidi = bidi;
        }

        @Override
        public void get(String url) {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public String getCurrentUrl() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public String getTitle() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public List<WebElement> findElements(By by) {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public WebElement findElement(By by) {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public String getPageSource() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public void close() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public void quit() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public Set<String> getWindowHandles() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public String getWindowHandle() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public TargetLocator switchTo() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public Navigation navigate() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public Options manage() {
            throw new UnsupportedOperationException("Not supported yet.");
        }

        @Override
        public Optional<BiDi> maybeGetBiDi() {
            return Optional.of(_bidi);
        }

    }
}

[pujagani]

Thank you so much for sharing this!

[github-actions[bot]]

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 14 days.