Message Flashing's implementation is broken and should be removed anyway

[mcilrain]

Message flashing as implemented in Flask is currently broken, under certain conditions it's possible for messages to be flashed to the wrong tab or in extreme cases the wrong device/browser.

A correct implementation would require functionality beyond Flask's scope, message flashing itself is arguably beyond Flask's scope already.

Why not make a piss-poor user account system that exists solely in session cookies? It seems like the same logic behind Flask's message flashing functionality.

[DasIch]

The claim that message flashing is broken and especially the claim that messages could be flashed to the wrong client is not at all obvious. Such a strong claim should be backed by argument, preferrably in the form of a reproducible test case. To understand what "broken" even means you should also describe exactly how you are using flashing, what you expect to happen and what is happening instead.

I personally think message flashing should be implemented as an extension, nevertheless it isn't and removing it, creates a backwards incompatibilty. This means that the cost of removing message flashing or rather moving the functionality to an extension is very expensive for the Flask community while the gain is purely aesthetic and therefore -- at least in this case -- neglible. In other words: it definitely should not be removed.

Furthermore a "user account system" is definitely outside the scope, especially if it is "piss-poor". The non-essential utilities Flask provides, come with the same dangers for the Flask ecosystem as the Python stdlib has for the entire Python ecosystem. These utilities will be used by everyone, even if a better solution exists and therefore are a potential danger to the code quality of any Flask project and innovation in the Flask ecosystem. We should keep the number of these utilities small, they should be of excellent quality and solve problems using solutions that have preferrably no potential for improvement.

[bigblind]

If I understand it correctly, the problem with the message flashing system is that it's purely cookie based. I can see how this could lead to problems, if a user opens a site in multiple different tabs. I haven't looked at the flash messages code, but the only way I can see flash messages going to the wrong device, is if there's something IP-matching based going on, with users behind a NAT.

[ThiefMaster]

Yeah, it's cookie-based. And I think the common use case is this:

1. (c) POST /foo/xxx
2. (s) flash(...)
3. (s) redirect(...)
4. (c) GET /foo/

So even with another tab open chances are very slim that a flash message ever goes to the wrong tab. Unless that other tab is reloading a page showing flash messages with a very high frequency. But in that case there's something else wrong with your user or your application...

[mcilrain]

Cookies can get synced between desktop browsers and mobile browsers if the
user has configured their browser to do so. If both devices are used
simultaneously it's possible a message will be flashed to the other device.

I have to wonder why a so-called microframework is dictating frontend design
decisions.

[untitaker]

I think the problem that appears with cookie syncing could be made less frequent by tying messages to a user agent, but even then problems might occur, and i think the feature (much less a perfect implementation of it) is out of scope for Flask. Removing this feature and writing a good extension as a drop-in replacement would be a great solution IMO.

From the README:

Consider the API to slightly improve over time but we don't plan on breaking it.

I don't consider removing a feature to be a breakage of the API, since message flashing is not essential to the usage of Flask.

[DasIch]

Cookies are the only reliable way to identify a client an HTTP client. You probably won't sync cookies on accident and if you do you should be aware of the consequences that has. It is also highly unlikely that someone is using multiple browsers syncing cookies, in such a way as to have flashed messages appear in the "wrong" browser.

If you update Flask and your app breaks because of changes to the Flask API, then there is breakage. That's the only definition that's useful and simple to understand and therefore the only definition worth using.

[mcilrain]

Are you personally willing to eradicate every instance of non-standard behavior? Good luck.

Why is a so-called microframework dictating frontend design decisions?

[jaapz]

I'd like to break in to this thread to say what great discussion skills you have, @mcilrain ...

[untitaker]

@DasIch:

Cookies are the only reliable way to identify a client an HTTP client.

I think there was a misunderstanding, i meant filtering the messages in get_flashed_messages() by whether the current UA matches the UA the message was originally sent to in a cookie, still using cookies as storage. I am pretty certain this doesn't solve enough problems, so it was rather brainstorming than anything else.

If you update Flask and your app breaks because of changes to the Flask API, then there is breakage.

Since every change to Flask's codebase could potentially break somebody's application, i am not sure if your definition is as useful as it is simple to understand. AFAIK we've also done some deliberate API changes during the Python 3 rewrite and after it, e.g. the get_data() thing on request objects.

@mcilrain:

Why is a so-called microframework dictating frontend design decisions?

I'm going to assume the repetition of that sentence wasn't deliberate, and i really hope to be right. While i agree with you on the original topic of this issue, i don't think Flask is "dictating" any frontend design decisions by providing a feature users may use or not.

[mcilrain]

I'm sorry I guess I thought this would be a clear open-shut case with the relevant code being immediately deprecated.

@jaapz, I guess you see it another way, would you mind enlightening me?

While i agree with you on the original topic of this issue, i don't think Flask is "dictating" any frontend design decisions by providing a feature users may use or not.

Then why shouldn't this support the inclusion of a bunch of other junk that doesn't work unless unrealistic assumptions are made about the client?

[untitaker]

@mcilrain Sorry, but i don't see your point. I agree that this feature is not well-implemented, and i agree that it is out of scope for Flask, and i agree that it should be removed. But, this feature is not really strongly tied to the rest of Flask, therefore allowing the user to ignore this "bad part" of Flask. If users are not required to use this feature in order to use Flask, how else could Flask then dictate any frontend design decisions?

[mcilrain]

The only part it's strongly tied to is the examples and I don't see why messages can't be passed as query arguments in that particular instance.

If this functionality has no reason to exist then it should not exist, I use a bunch of snippets that are just as (if not moreso owing to lack of bugs) appropriate for inclusion into the flask library.

You have to draw the line somewhere and I think it should be drawn at the part that says "poorly written front-end functionality shouldn't exist in a microframework".

[untitaker]

Putting the messages into the URL introduces similar problems, for example, one could bookmark the homepage of some onlineshop after making a purchase and later freak out because the success message is shown a second time.

Also, keep in mind that no participant here is thinking that having message flashing in Flask is a good idea, but at the same time we have to keep backwards compatibility in mind, about which i am concerned too (not as much as @DasIch though)

[mcilrain]

You misunderstand, I am not suggesting that Flask's implementation switch to using query arguments, I am suggesting that it be removed entirely and that Flask's example code should use query arguments instead of Flask's broken message flashing implementation, using query arguments in this way is bad practise, but so is message flashing. This solves the dependency problem.

Am I not the only one who finds it disingenuous that Flask's message flashing functionality is extensively used in the examples?

[ThiefMaster]

and I don't see why messages can't be passed as query arguments in that particular instance

ESPECIALLY in an example bad approaches like the one you suggested are horrible. A tutorial/example is likely to be read by inexperienced developers who don't know that query arguments are a bad idea.

Also, I think message flashing is a very useful feature even in a microframework. Displaying a result/error/warning message right after a redirect is a common use-case, especially for small-ish applications.
However, an official extension would make sense for it - especially since it would hopefully be referenced in the documentation when the builtin flashing is removed at some poing.

[mcilrain]

ESPECIALLY in an example bad approaches like the one you suggested are horrible. A tutorial/example is likely to be read by inexperienced developers who don't know that query arguments are a bad idea.

That's an argument for not having message flashing at all. I was merely offering a solution to a dependency problem, ideally message flashing should be removed entirely as it is a flawed

Displaying a result/error/warning message right after a redirect is a common use-case, especially for small-ish applications.

If it's something the user needs to be aware of then using Flask's message flashing functionality is wrong due to its unreliability. It should be used exclusively for things that user doesn't need to know about.

Proven implementations (or at least reliable implementations without known bugs) should be used in instances where the delivery needs to be guaranteed.

Flask's message flashing functionality seems to exist solely to produce attractive example code, while I think moving it to an extension is a great idea it would undermine its original purpose of misrepresenting Flask's functionality.

[groteworld]

That's an argument for not having message flashing at all. I was merely offering a solution to a dependency problem, ideally message flashing should be removed entirely as it is a flawed

I assume the dependency issue you're talking about is the examples being dependent on flashing. If I'm wrong on that assumption, please let me know. I think the problem with giving no flashing or equivalent functionality in examples/flask is the fact that inexperienced developers will make, arguably, worse implementations.

While I understand the only thing that's going to make you happy is a total deletion of flashing, IMO the special case you're describing doesn't feel warranted for a removal of a well-used feature. If it doesn't work for your system, you can always override or not use it. right?

Maybe just add to the flask docs warning of unreliability of flash message retrieval?

[DasIch]

Could we please stop with the hyperbolic bullshit? Message flashing in Flask can produce a possibly unwanted result in exactly two cases:

1. The user makes a request producing flashed messages in tab A.
2. The browser receives a response (most likely a redirect) with the flashed messages.
3. The user makes a request in another tab B before or in parallel with the request that will be made from tab A, to a page that displays flashed messages.
4. The server answers the request from tab B before the (likely occurring) request from tab A.
5. Flashed messages appear in B instead of A.

I dare you to actually manage triggering this case, I suspect that in almost all cases the timeframe you have to trigger this case, is shorter than your reaction time.

The second far more interesting case:

1. The user makes a request producing flashed messages in browser A.
2. The browser receives a response (most likely a redirect) with the flashed messages.
3. The browser syncronizes the received cookie with browser B (which I suspect is not going to happen after every request for a huge number of good reasons.)
4. The user makes a request in browser B, to a page that displays flashed messages.
5. The server answers the request from browser B before the (likely occurring) request to browser A.
6. Flashed messages appear in B instead of A.

Again I would like to dare you to try triggering this case.

I would also like you to consider that it might be more important not to lose the information in a flashed message and would therefore want to display messages to the user, even if that's not possible on the page the user was redirected to.

The latter part becomes really interesting, if we consider the case of a user stopping the usage of one device and continues to use the a web page on another. This might occur, if someone leaves their home and continues to use a page from a phone instead of their desktop computer or if someone merely moves from their desk to their sofa on which they prefer to use a tablet. In these cases a seamless transition is desired and this might very well include displaying flashed messages, which might signal a failure of some action, to which the user might want to react.

This is not at all a case of broken or not, this is a case of trade-offs in complexity and possible use cases. The implementation Flask uses, has sane, easily explained and consistent behaviour. There is nothing broken or bad about it nor is it at all clear that it shouldn't be used.

I'm not opposed to breaking backwards compatiblity, quite the opposite in fact. Nevertheless if you do that, you should have a good reason. Introducing Python 3 support was one such reason, the purely theoretical case of someone perfoming a request when using a Flask application, within a certain timeframe possibly a few hundred ms long and then may be getting flashed messages at a different location than possibly intended, is anything but a good reason.

This has devolved to what is at best an exercise in mental masturbation over some theoretical aesthetically perfect solution to flashing messages that nobody has even come up with. So unless someone can actually show that is is indeed a problem and not just a theoretical one and that it can be reasonably solved just please stop this bullshit.

[mcilrain]

All those words and not a single one to justify its existence. Take a look at the manual's quickstart section, look at how inappropriate its inclusion seems!

“Micro” does not mean that your whole web application has to fit into a single Python file, although it certainly can. Nor does it mean that Flask is lacking in functionality. The “micro” in microframework means Flask aims to keep the core simple but extensible. Flask won’t make many decisions for you, such as what database to use. Those decisions that it does make, such as what templating engine to use, are easy to change. Everything else is up to you, so that Flask can be everything you need and nothing you don’t.

But there's message flashing functionality that will mostly go unused built right in because otherwise example snippets won't look clean.

[groteworld]

Flask won’t make many decisions for you...

and no one is forcing you to use message flashing. Roll your own if you need better, and if it's really good, contribute to the repo. At your same argument Flask shouldn't have functions to use Jinja templates, because if I don't use Jinja, then its just wasted code that I don't need.

But there's message flashing functionality that will mostly go unused...

Do you have statistics on flash usage?

To just remove because YOU don't use this implimentation with no replacement, be that in core or an extension, seems nitpicky.

[TronPaul]

Such a strong claim should be backed by argument, preferrably in the form of a reproducible test case. To understand what "broken" even means you should also describe exactly how you are using flashing, what you expect to happen and what is happening instead.

I'd like to reiterate on this. Could you please write a test case where this issue is reproducible? I'm curious what such a test looks like.

[euank]

I've used a site with the message flashing problem being both frequent and easy to replicate. The site in question was a file upload site. Users could upload files, which were then processed by the backend. This processing often took 1-3 seconds. In that time a user could easily open another tab to browse another portion of the site and lo and behold, that tab would have the "File uploaded successfully" message.

It's perhaps ironic that such messages are easily reproducible when there's very heavy backend processing because the developer will most often want to display a message after heavy processing - the user needs to be reassured after any slow or error-prone process.

I do not have a test case to provide, but I believe this could help in producing one; simply add a sleep to simulate a complex join or other slow process.

[ThiefMaster]

Actions that cause a full page reload (i.e. non-AJAX calls) shouldn't trigger synchronous "heavy processing" in the first place. It's horrible UX. However, I don't see how this can cause the problem you mentioned. The cookie is not sent to the client until the request finished (unless you stream a response) - so working in another tab in the meantime won't catch the flash.

[mcilrain]

Message flashing is arguably bad UX too.

Why is there an argument about UX in Flask's issue tracker? Flask shouldn't have anything to do with UX but it's inclusion of message flashing means it does.

The solution is obvious.

[groteworld]

The solution is obvious.

You're right. I move to close discussion on issue 925.

[cbcafiero]

I second the motion.

On 2013-12-17, at 03:44 PM, Blake Grotewold notifications@github.com wrote:

The solution is obvious.

You're right. I move to close discussion on issue 925.

—
Reply to this email directly or view it on GitHub.

[mcilrain]

This seems to be a contentious issue, I think a better course of action would be to remove the offending code and consider it's reinclusion.

It's an unfortunate fallacy to want undesirable things if the alternative is losing them, the prospect of change is very frightening to some people.

The only rational course of action would be to remove the offending code and consider its re-inclusion.

[clintecker]

Just wondering out loud, but I'm curious whether or not you might've had more success here if you hadn't been so aggressive and combative.

[bigblind]

@mcilrain Wouldn't it be better in that case to act-as-if? to ask weather
we'd want to include it if it wasn't there, and someone made a pull
request, offering to put it in? If we can change the way we think about it
without changing the code, we don't have to make backwards incompatible
changes before we've actually made a decision.

2013/12/17 Clint Ecker notifications@github.com

Just wondering out loud, but I'm curious whether or not you might've had
more success here if you hadn't been so aggressive and combative.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/925#issuecomment-30796220
.

[mcilrain]

It seems the consus is that the implementation is broken, but it's only a problem if used with requests that take long enough for the user to interact with the Flask application in another tab or window.

I see several problems with that, it makes assumptions about the way the backend operates in all conditions, it makes assumptions about the network conditions between the user and the server, it makes assumptions about how the browser functions and it makes assumptions about the way the user interacts with the web.

So as long as the developer is aware of all the things (present and future) that could potentially break cookie-based message flashing it works fine.

Because a non-broken implementation would require persistence which is out-of-scope for the core Flask module it needs to remain the same, be removed entirely or require Flask to take on dependencies and/or bloat and will require additional configuration.

I vote to remove it entirely. I'm happy with it being in an extension if there's enough desire for it but considering its tiny footprint maybe it works better as a snippet people can copy/paste.

[mcilrain]

@bigblind Obviously I am suggesting that the functionality be deprecated and removed at a later date, not ripped right out without any consideration of those who may be using it. I'd argue that keeping it in is substantially more inconsiderate as they may be using that functionality under the pretense that it works properly under all conditions.

[doobeh]

Message flashing is well used and at the same time easily ignored if you require a more robust solution for your edge case.

If you are arguing that it shouldn't be used in the tutorial documentation, that is a separate issue, which can be handled simply by stating your case in a non-aggressive way, and submitting patches to the documentation that show an equally easy to understand, yet more robust approach which in turn will allow it's usage to naturally deprecate.

[naringas]

I, for one, use message flashing in may application and would not like to see it removed simply because its implementation could be better.

-1

[thibaultmeyer]

I agree with @naringas. I use too flash messages on my flask projects... And removing it simply because one guy on earth said that implementation could be better and refuse to use it is just a foolish decision...

[mitsuhiko]

This discussion is pointless. The tradeoffs of the message flashing implementation are well known and intentionally made when the function was written. It exists so that simple applications can give good user feedback without having to implement a much more complex system. Removing this function would not do any good.

The problem is so incredibly far fetched that it's just not worth discussing.