If an exception is thrown in a tile, @@updatetilecontent gives an error 500 and nothing is shown to the user

[idgserpro]

This is difficult to reproduce, since we're using a portuguese tile component in a complex setup that has this behavior, so we'll try to simulate it editing some of collective.cover implementation files to easy the test:

1 - Clone this repository (we tested using commit 6d615de, before 1.0a13 release);
2 - python bootstrap.py and bin/buildout;
3 - Create a simple cover and add a simple tile;
4 - Stop Plone;
5 - Edit src/collective/cover/tiles/basic.py and add a bunch of

raise Exception("Testing cover handling of tiles brokenness")

to all methods (or choose any method that's called in compose view when you select a content to add);

6 - Start Plone again;
7 - You should see:

This is not a helpful message, but at least the cover isn't broken (similar message when you have a broken portlet);
8 - Go to compose;
9 - Try to add a new item to the basic tile that throwns the exception;
10 - You get:

Infinite loading and a 500 Internal Server error, so it's not possible to the user to know if an error occured or if the server is slow. When you click compose again, you see that nothing was saved.

I'm not familiar with this component source code, neither it's javascript implementation, but I think a message should be shown to the user telling an error on the server happened: don't know the best place to show it. Maybe in the place of the loading icon?

[hvelarde]

sounds fair to me; @rodfersou let's take a quick look on this

[idgserpro]

We can add a failure: function() to all ajax calls in

collective.cover/src/collective/cover/static/js/contentchooser.js

Line 490 in adb1a67

url: '@@updatetilecontent',

, or statusCode to handle it... but I don't know if there's a better approach. In my opinion, if possible in jquery, a nice thing to do would be a generic handling of all 500 errors.

Since we have .loading-mask, we can create an .error-mask or something equivalent, that would be used in one of the ideas above to warn the user that something went wrong on the server.

@rodfersou and @hvelarde, ideas?

[hvelarde]

+1

[idgserpro]

@hvelarde is 1.0a13 towards Plone 5.0? Should I add plone.app.stagingbehavior to packages that depend on collective.cover that will still be installed in 4.3?

[hvelarde]

yes, the idea is to fix all compatibility problems, one step at a time, until we get the package running in both versions.

[rodfersou]

@idgserpro for me it is fair to add a failure method and send the error message there.

But there are other places where we need these failure treatment:

• collective.cover/src/collective/cover/static/js/contentchooser.js

  Line 490 in adb1a67

  url: '@@updatetilecontent',

• collective.cover/src/collective/cover/static/js/contentchooser.js

  Line 453 in adb1a67

  url: '@@movetilecontent',

• collective.cover/src/collective/cover/static/js/contentchooser.js

  Line 469 in adb1a67

  url: '@@updatetile',

• collective.cover/src/collective/cover/static/js/compose.js

  Line 71 in adb1a67

  url: "@@updatelisttilecontent",

And not sure if we need this here:

• collective.cover/src/collective/cover/static/js/compose.js

  Line 30 in adb1a67

  url: "@@removeitemfromlisttile",

Need to look with more attention everywhere.

For me the error message could be placed in the same place there is the message "Tile content replaced during the blocks transform." and we think about another kind of mask, or style when there is and error.

@agnogueira could you please think about the style of the error message?