Failure in workflows cause the message to be displayed twice

[Julie Pichon]

The handle() method for worflows expects a boolean to be returned to indicate if the workflow completed successfully, however returning False results in two error messages being displayed.

https://github.com/openstack/horizon/blob/f4282f73ec/horizon/workflows/base.py#L808

You can reproduce by causing an error somewhere where we use the handle() method and return False. For example:

1. Go to the Access & Security page
2. Click on Allocate IP
3. Turn off the Neutron service (or nova-network if using it for networking)
4. Click on "Allocate IP" in the modal: 2 error messages are displayed ("Error: Connection to neutron failed: Maximum attempts reached" and "Error: Unable to retrieve floating IP pools.")

 https://github.com/openstack/horizon/blob/f4282f73ec/openstack_dashboard/dashboards/project/access_and_security/floating_ips/workflows.py#L143

I suspect it's because both exceptions.handle() and the workflows call on messages.error():

https://github.com/openstack/horizon/blob/f4282f73ec/horizon/workflows/views.py#L200
https://github.com/openstack/horizon/blob/f4282f73ec/horizon/exceptions.py#L215

[Facundo Farias (facundo-farias)]

So, it should be handled by the view or by the exception handler?

[Akihiro Motoki (amotoki)]

The same issue is reported in bug 1289496.
This bug report has more detail information and it seems better to be tracked in this bug.

[Julie Pichon (jpichon)]

A not uncommon dilemma with duplicate bugs :-) Using this bug here to track the 2 error messages, and the other one to improve the error message wording, makes sense to me. We probably want to update the description of the other one if we do it this way.

Facundo: I guess so, it'll be up to whoever picks up this bug to investigate which option makes more sense and what are the implications for each (e.g. making sure this doesn't make us lose error messages in other contexts).

[Cindy Lu (clu-m)]

Another bug too: https://bugs.launchpad.net/horizon/+bug/1278353

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/116226
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=a212ef399f78633d7627b633f733f8c525c30aa3
Submitter: Jenkins
Branch: master

commit a212ef399f78633d7627b633f733f8c525c30aa3
Author: Vlad Okhrimenko <email address hidden>
Date: Fri Aug 22 14:04:16 2014 +0300

    Uniquify horizon messages returned in a single response

    A network operation can fail because of different
    reasons - yet in many places just one error message
    is provided. Combined with too broad exception clause,
    and incorrect assumptions on the reasons of failure
    (e.g. Neutron service being unavailable causes all
    other sorts of errors like inability to Allocate IP
    or Associate it) this leads to multiple errors when just one
    would suffice. The fix aims to provide sensible error
    messages in case the network service is down. This includes
    returning only unique messages in a single response.

    Partial-Bug: #1301374
    Change-Id: Id6c620ca51f7703f35c0c172e39fdf237fa42278
    Co-Authored-By: Timur Sufiev <email address hidden>

[Julie Pichon (jpichon)]

Vlad: Thank you for the partial fix! Are you still working on this or should we revert the bug status back to "Confirmed" for someone else to tackle the rest?

[Vlad Okhrimenko (vokhrimenko)]

I fixed this bug (https://review.openstack.org/#/c/116226/)
But i still work on https://bugs.launchpad.net/horizon/+bug/1289496

[Julie Pichon (jpichon)]

Thanks! The review you linked to was marked as a "Partial Fix" in the commit message, which seems correct to me since multiple error messages still happen under other circumstances. That's why I'm wondering about what else is planned to resolve it?

[Vlad Okhrimenko (vokhrimenko)]

Yes of course. I'm working on it

[Julie Pichon (jpichon)]

Sweet, sorry for the noise so :)

[Timur Sufiev (tsufiev-x)]

Since bug 1289496 is also already fixed, I guess we can finally close this one.

[Julie Pichon (jpichon)]

Timur, it looks like these patches fixed the problem for specific occurrences rather than generically for all workflows though, is that correct?

[Timur Sufiev (tsufiev-x)]

Julie, having looked more carefully through the Bug Description, I've realized that you're right and I had closed this one prematurely. I have at least one solution in mind, which I'll try to implement. Thus reverting the status to In Progress.

[Julie Pichon (jpichon)]

Great to hear, thank you Timur!

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/140708

[Timur Sufiev (tsufiev-x)]

Julie, I've tried to as explanatory as I could in the commit message, but perhaps the above picture could explain the things better. It is produced for the Neutron service being shut down just before pressing the final 'Create' button at the 'CreateNetwork' workflow. Here you see 2 error messages generated for the `CreateNetwork` workflow which has `show_multiple_failures` set to `False`. The reason there are 2 messages is that they are generated for different request-response cycles: the first long message gets into request before this line https://github.com/openstack/horizon/blob/2014.2.1/horizon/workflows/views.py#L195 is executed, while the second is added right after the redirect. If I clear messages just before `exceptions.handle(request)` line, there won't be any specific message about the cause of the unexpected error (which is not recoverable).

So I propose to reduce the number of messages only for recoverable (expected) errors, because it's not clear to which error message we should reduce a bunch of them in case they were caused by some unexpected failure.

[Timur Sufiev (tsufiev-x)]

Seems that the solution to this issue is a thing nobody currently cares about, especially given trend to Angularization.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Timur Sufiev (<email address hidden>) on branch: master
Review: https://review.openstack.org/140708
Reason: Looks like nobody is interested in this patch, abandoning - feel free to restore if you're interested in it.