Handling mid-stream errors #1
Conversation
|
|
||
| In the past, browsers other than Internet Explorer accepted `multipart/x-mixed-replace` responses to be loaded in `target="_top"`, and that could potentially be used to swap to a a new copy of the response-in-progress but with error-indicating headers, or potentially even a replacement error page. However, modern browsers have quietly dropped support for `multipart/x-mixed-replace` in the top browsing context. | ||
|
|
||
| It _might_ be possible to write `<meta http-equiv="refresh" content="0;url=${errorPageLocation}">`. It’s not _supposed_ to be outside the `<head>`, but I still like it better than how Rails creates a mid-stream error redirect with an inline `<script>`. ([some context on how search engines could better understand this is a temporary redirect, not a permanent one](https://twitter.com/JohnMu/status/969486943351394304)) |
There was a problem hiding this comment.
FWIW this is what I've seen in several applications. Curious if this is one of those "kinda not supposed to work but does" things.
There was a problem hiding this comment.
It definitely works for browsers and probably search engines, but caches still probably need the explicit indicator that the HTTP response was incomplete.
There was a problem hiding this comment.
One thing some of the prior art has handled partially is what happens if the stream error occurs while the HTML has already opened a tag, attribute value, etc. Rails prepends "> and I’ve also seen </script>
GitHub explored this problem even more thoroughly for Dangling Markup Busting in their Post-CSP journey
There was a problem hiding this comment.
Spitballing here:
out.streamErrorRedirect = function (url) {
out.shouldMarkAsIncomplete = true;
out.write(`
<!--'"--><!--]]></style></script></textarea></select></template>-->
<meta http-equiv=refresh content="0;url=${url}">
<script${out.global.cspNonce ? ' nonce='+out.global.cspNonce : ''}>location=${JSON.stringify(url)}</script>
<a href=${url}>Follow this link if you aren’t redirected</a>
`);
};-
Should there be a default error page URL taken from
out.global.error500Url, likeout.global.cspNonce? -
How much should Marko protect against dangling markup? Rails and other existing solutions don’t seem to care much.
-
Writing further error content should still happen with whatever is inside
<@catch>, in case both the inline<script>andmeta[http-equiv=refresh]fail (not likely, but certainly not impossible.) Should it go before or after the stream error redirect markup? My vote is for before.
There was a problem hiding this comment.
@tigt I might've missed this in another part of this discussion, but is there value in writing both the meta and script tags here? In which case would one work but not the other? I feel like meta is more practically reliable and works with js disabled which is a bonus.
There was a problem hiding this comment.
I'll also add that having dangling markup is probably very rare since for the most part things are escaped. To have dangling markup it would mean that you would have gotten your application into a state where you are passing invalid markup to an unescaped interpolation which is an issue in and of itself.
There was a problem hiding this comment.
I don’t know of a specific browser where it happens, but my intuition makes me worry about relying solely on the option that is technically invalid HTML — all it would take is one high-profile unescaped markup attack to make some security-conscious browser ignore meta refreshes in the <body>. (Maybe Tor Browser or Brave already do.)
There was a problem hiding this comment.
Blast from the future: turns out there might be value in writing both meta[http-equiv="refresh"] and window.location=. WebKit changeset 280,870:
Meta HTTP refresh should not navigate if document has sandboxed automatic features browsing context flag set:
Firefox and Chrome already behave this way.
I haven’t managed to figure out what the spec means by that yet, but putting it here for later.
|
|
||
| - Express and other Node HTTP frameworks | ||
| - https://expressjs.com/en/guide/error-handling.html#the-default-error-handler | ||
| - https://github.com/expressjs/express/issues/2700 (!!!) |
There was a problem hiding this comment.
This is interesting. If express's next/error handler does support this properly then perhaps using the marko/express plugin should be good enough. We actually forward Marko's errors to express in that case.
There was a problem hiding this comment.
Only uncaught errors though. The case we need to think about is when the error was caught and so we were able to render a page for the user, but we want to signal that content is missing to any proxies/bots/etc. that might otherwise cache/index a page that didn't render all its content.
If we did want to rely on this for those cases, we'd probably have to hold off on firing the event until just before Marko ends the stream, otherwise express would close the connection before we were done rendering. We'd also probably want to use an event other than error since I know there's already apps out there listening for that event and doing a script/meta redirect.
|
Yeah, especially since real HTTP errors don't change the URL address. Maybe
redirecting isn't a good pattern in the first place; it's certainly
unexpected for users, and there isn't a valid, clean way to express it in
the bedrock HTML or HTTP layers.
…On Sun, Apr 12, 2020, 12:13 PM Dylan Piercey ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In proposals/0000-surface-async-stream-errors-to-http.md
<#1 (comment)>:
> + <dt>HTTP/1.1 <code>Transfer-Encoding: chunked</code></dt>
+ <dd><a href="https://httpwg.org/http-core/draft-ietf-httpbis-messaging-latest.html#incomplete.messages">IETF Draft: HTTP/1.1 Messaging §8 Handling Incomplete Messages</a></dd>
+ <dd><p>If a chunked response doesn’t terminate with the zero-length end chunk, the client must assume that the response was <i>incomplete</i> — which at the very least, <a href="https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.3.1">means a cache should double-check with the server before reusing the stored incomplete response</a>.</p></dd>
+
+ <dt>HTTP/2 Streams</dt>
+ <dd><a href="https://tools.ietf.org/html/rfc7540#section-5.4.2">RFC 7540 §5.4.2 Stream Error Handling</a></dd>
+ <dd><p>An HTTP/2 stream can signal an application error by sending a <code>RST_STREAM</code> frame with an error code of <code>0x2 INTERNAL_ERROR</code>.</p></dd>
+</dl>
+
+This proposal explores how Marko should surface errors from server components to the HTTP layer, so the response can properly signal an error with one of the above methods.
+
+## Guide-level explanation
+
+Developers should be able to tell Marko how important an async error is:
+
+1. Our example from the Motivation section is as serious as it gets — you might even want to redirect to a completely different error page
I guess one work around would to be store the original page url and do
some magic if the referrer is the error page itself to navigate back. Seems
a bit strange though.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB5S2SVZNRJNN3OJ5OH2KADRMHSENANCNFSM4MDVSGFQ>
.
|
|
This is how I’m leaning after thinking for a few days: I don’t want to encourage mid-page redirects
If the intent of existing implementations (e.g. Rails) is to have a fallback generic error handler/message for the user, we can achieve that with some API for uncaught Handling mid-stream errors is something Marko should have docs forThe fact that I had to do quite a bit of research for a problem that’s existed since 1997 suggests that this is information Marko authors could certainly use.
|
|
It also seems like Marko could know if an underlying stream is for Node’s
It seems like |
…-surface-stream-errors-to-http.md
tigt
changed the title
Show-stopping errorsIt is highly recommended not to begin streaming a Marko response (or any HTTP response) for crucial page content, such as:
In these cases, you should wait to begin the HTTP stream until you know which status code to send. If you want to begin streaming ASAP for performance reasons, consider the following:
Trivial errorsDo nothing different. If you don’t care that the “recommended for you” section renders or not, Something in-betweenThis is where advice for listening to the Marko stream’s Responsibilities that Marko can help developers with:
|
|
Related: I found an old Apache bug ticket on correctly forwarding malformed CTE streams: https://bz.apache.org/bugzilla/show_bug.cgi?id=55475 Lots of relevant discussion, URLs, and good examples of running code in there. |
|
I pinged the HTTP working group at httpwg/http-core#895, and that thread contains a few links and a suggestion to bring this problem area to the httpwg mailing list |
No description provided.