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

NPE related to sorting when recreating columns #1969

Closed
vesanieminen opened this issue May 15, 2019 · 10 comments · Fixed by vaadin/flow-components#468, #2122 or vaadin/web-components#270
Closed

Comments

@vesanieminen
Copy link

The following error happens if Grid columns have sorting enabled and the columns are recreated:

Caused by: java.lang.IllegalArgumentException: Received a sorters changed call from the client for a non-existent column at com.vaadin.flow.component.grid.Grid.sortersChanged(Grid.java:2791)

Screen Shot 2019-05-15 at 10 44 44

Steps to reproduce:

  1. git clone [email protected]:vesanieminen/Vaadin-Grid-recreate-columns-bug.git
  2. mvn jetty:run
  3. navigate to localhost:9997
  4. click on the sorting buttons on the grid header for one of the columns
  5. Select a different value from the Select component.

Expected behaviour:

  • The Grid columns are recreated with no exceptions in the backend

Actual behaviour:

  • The Grid columns are recreated with an exception.

Workaround exists here: vesanieminen/Vaadin-Grid-recreate-columns-bug@100d56c

Full error log for the bug:

[qtp1571205437-69] ERROR com.vaadin.flow.server.DefaultErrorHandler - java.lang.RuntimeException: java.lang.IllegalArgumentException: Received a sorters changed call from the client for a non-existent column at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:174) at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:129) at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.handleNode(PublishedServerEventHandlerRpcHandler.java:117) at com.vaadin.flow.server.communication.rpc.AbstractRpcInvocationHandler.handle(AbstractRpcInvocationHandler.java:64) at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocationData(ServerRpcHandler.java:387) at com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$1(ServerRpcHandler.java:368) at java.util.ArrayList.forEach(ArrayList.java:1251) at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocations(ServerRpcHandler.java:368) at com.vaadin.flow.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:310) at com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:89) at com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40) at com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1507) at com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:242) at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:865) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1655) at org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter.doFilter(WebSocketUpgradeFilter.java:215) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1642) at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:533) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:146) at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:548) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132) at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:257) at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1595) at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255) at org.eclipse.jetty.server.handler.ContextHandler.__doHandle(ContextHandler.java:1317) at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:42020) at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:203) at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:473) at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1564) at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:201) at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1219) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:144) at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:219) at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:126) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132) at org.eclipse.jetty.server.Server.handle(Server.java:531) at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:352) at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:260) at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:281) at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:102) at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:118) at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333) at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310) at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168) at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:126) at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:366) at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:762) at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:680) at java.lang.Thread.run(Thread.java:745) Caused by: java.lang.IllegalArgumentException: Received a sorters changed call from the client for a non-existent column at com.vaadin.flow.component.grid.Grid.sortersChanged(Grid.java:2791) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.__invoke(DelegatingMethodAccessorImpl.java:43) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:45009) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:45012) at java.lang.reflect.Method.invoke(Method.java:498) at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:167) ... 49 more

@mperktold
Copy link

We experience the same problem in our project.

Our use case is to switch between one set of displayed columns to another set of columns when the viewport width changes beyond a certain threshold, to implement responsive tables.

As there is currently no API for inserting new columns at a certain index between existing ones, the only way is to remove all columns of the old set and add the ones of the new set.

I tried the workaround you suggested, but it didn't work in my case.
The following workaround works better, but still does not handle all cases.

It follows another approach: Your's is to temporarily reset the order and disable sorting for all columns, mine is to tell the client to avoid sending sorters-change events to the server while columns are being updated.

So on the server side, I have a custom Grid implementation:

@Tag("vaadin-custom-grid")
@HtmlImport("src/grid/custom-grid.html")
public class ExtendedGrid<T> extends Grid<T> implements KeyNotifier {

    @ClientCallable
    private void refresh() {
        getDataProvider().refreshAll();
    }

    @ClientCallable
    private void clientUpdatedColumns() {
        getElement().setProperty("_updatingColumns", false);
    }

    public void setUpdatingColumns() {
        getElement().setProperty("_updatingColumns", true);
    }
}

With the following client-side implementation:

class GridElement extends Vaadin.GridElement {

    static get is() {
        return "vaadin-custom-grid";
    }

    static get properties() {
        return {
            _updatingColumns: {
                type: Boolean,
                value: false,
                observer: "_updatingColumnsChanged"
            }
        };
    }

    _updatingColumnsChanged(value, oldValue) {
        if (value)
            this.$server.clientUpdatedColumns();
        else
            this.$server.refresh();
    }

    _onSorterChanged(e) {
        if (!this._updatingColumns)
            super._onSorterChanged(e);
    }
}

The server must call setUpdatingColumns() before recreating columns to activates a corresponding boolean flag on the client side.

As long as this flag is activated, the client ignores all sorter-changed events to prevent the error.
Also, when the client activates the flag, it tells the server about it.
The idea is to keep the flag active for one roundtrip, and to reset it afterwards.

Finally, when resetting the flag, we refresh the data.
I don't know why, but I needed this to prevent ending up with an empty table page after changing the columns.

Now this whole approach works fine for me, except when there is an open Vaadin Dialog while changing columns. Then I still get the same error.

I have no idea how the dialog relates to the grid columns, but this is what I am experiencing.

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-grid-flow Oct 6, 2020
@vaadin-bot vaadin-bot added the flow label Oct 6, 2020
@mperktold
Copy link

In the meantime, we now use Grid.addColumn, Grid.removeColumn, and Grid.setColumnOrder to update the only those columns that actually changed.
That seems to improve things, but the error still appears from time to time.

@KasparScherrer
Copy link

I'm running into this issue as well, basically whenever i call removeColumn on a column that was sortable (and the user actually sorted it). The two workarounds suggested here aren't doing it for me.

This leads to me creating a new instance of my grid whenever i would remove a column. I really don't like this.
Therefore i wish the underlying bug would be fixed so that when I remove any column, the frontend-part stops trying to sort a column that no longer exists.

@tomivirkki
Copy link
Member

The issue is likely caused by vaadin/vaadin-grid#1691

@szlac71
Copy link

szlac71 commented Feb 5, 2021

			for(Column<T> col : grid.getColumns()) {
				col.setSortable(false);
			}
			grid.sort(null);

@dskbrd
Copy link

dskbrd commented Mar 18, 2021

I ran into the same problem when removing a sorted column and I think I found a possible workaround by manually clearing the "_sorters" referenced by the frontend-component before removing any column:

// Clear any previous sorting
grid.sort(null);
// Explicilty clear sorting referenced in frontend
grid.getElement().executeJs("this._sorters = []");
// Remove column from grid
grid.removeColumnByKey(fieldName);

This seems to fix the issue but I am not yet sure what other side effects this may cause.

@tulioag tulioag assigned tulioag and unassigned yuriy-fix Apr 12, 2021
@tulioag tulioag reopened this Apr 27, 2021
web-padawan pushed a commit to vaadin/web-components that referenced this issue Apr 28, 2021
…/ detached (#270)

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Yuriy Yevstihnyeyev <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>
vaadin-bot pushed a commit to vaadin/web-components that referenced this issue Apr 28, 2021
…/ detached (#270)

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Yuriy Yevstihnyeyev <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>
web-padawan pushed a commit to vaadin/web-components that referenced this issue Apr 28, 2021
…/ detached (#270) (#274)

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>
Co-authored-by: Yuriy Yevstihnyeyev <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>
tulioag added a commit that referenced this issue Apr 28, 2021
fix: update filtering and sorting when grid and columns are attached / detached (#2122)

Fixes: #1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>
manolo pushed a commit to vaadin/flow-components that referenced this issue Apr 28, 2021
tulioag added a commit that referenced this issue Apr 30, 2021
…ttached / detached (#2122) (#2172)

cherry-pick: update filtering and sorting (#2122)

fix: update filtering and sorting when grid and columns are attached / detached (#2122)

Fixes: #1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Sascha Ißbrücker <[email protected]>
tulioag added a commit to vaadin/flow-components that referenced this issue May 4, 2021
* fix: Prevent IllegalArgumentException when sortable columns are removed and recreated

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns 

Co-authored-by: Manolo Carrasco <[email protected]>
Co-authored-by: Tulio Garcia <[email protected]>
vaadin-bot pushed a commit to vaadin/flow-components that referenced this issue May 4, 2021
* fix: Prevent IllegalArgumentException when sortable columns are removed and recreated

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns 

Co-authored-by: Manolo Carrasco <[email protected]>
Co-authored-by: Tulio Garcia <[email protected]>
tulioag added a commit to vaadin/flow-components that referenced this issue May 6, 2021
* fix: Prevent IllegalArgumentException when sortable columns are removed and recreated

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>
manolo added a commit to vaadin/flow-components that referenced this issue May 7, 2021
)

* fix: Prevent IllegalArgumentException when sortable columns are removed and recreated

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns 

Co-authored-by: Manolo Carrasco <[email protected]>
Co-authored-by: Tulio Garcia <[email protected]>

Co-authored-by: Tatu Lund <[email protected]>
Co-authored-by: Manolo Carrasco <[email protected]>
Co-authored-by: Tulio Garcia <[email protected]>
manolo pushed a commit to vaadin/flow-components that referenced this issue May 7, 2021
…ed and recreated(#468)(#918) (CP 19.0)  (#931)

* CP: Prevent IllegalArgumentException (#468)

* fix: Prevent IllegalArgumentException when sortable columns are removed and recreated

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>

* chore: fix duplicate element id in RemoveSortableColumnsIT (#918)

Co-authored-by: Tatu Lund <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>
tulioag added a commit to vaadin/flow-components that referenced this issue May 7, 2021
fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5)  (#931)

CP: Prevent IllegalArgumentException (#468)

fix: Prevent IllegalArgumentException when sortable columns are removed and recreated

Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>

* chore: fix duplicate element id in RemoveSortableColumnsIT (#918)

Co-authored-by: Tatu Lund <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>
tulioag added a commit to vaadin/flow-components that referenced this issue May 10, 2021
cherry-pick: #468 
cherry-pick: #918 
fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5)  (#931)
Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns
vaadin-bot pushed a commit to vaadin/flow-components that referenced this issue May 10, 2021
cherry-pick: #468 
cherry-pick: #918 
fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5)  (#931)
Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns
vaadin-bot pushed a commit to vaadin/flow-components that referenced this issue May 10, 2021
cherry-pick: #468 
cherry-pick: #918 
fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5)  (#931)
Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns
ZheSun88 pushed a commit to vaadin/flow-components that referenced this issue May 10, 2021
cherry-pick: #468 
cherry-pick: #918 
fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5)  (#931)
Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>
sissbruecker pushed a commit to vaadin/flow-components that referenced this issue May 10, 2021
cherry-pick: #468 
cherry-pick: #918 
fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5)  (#931)
Fixes: vaadin/vaadin-grid#1969
Warranty: Fixes exception related to sorting when recreating columns

Co-authored-by: Tulio Garcia <[email protected]>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 19.0.7. For prerelease versions, it will be included in its final version.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 20.0.0.beta2. For prerelease versions, it will be included in its final version.

@cmrgKoradir
Copy link

@dskbrd I don't suppose you remember, but in case anybody is reading this, is there a side-effect to this workaround we should be aware of?

Because as it turns out, we can still run into this in vaadin 24.2.

@web-padawan
Copy link
Member

@cmrgKoradir If you have a code example to reproduce the problem, please submit a new issue and we will look into it.
Also please check if it's valid with a more recent version, preferably 24.4 or 24.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment