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

preview server: revert back to blaze backend for better user experience #311

Closed
wants to merge 3 commits into from

Conversation

jenshalm
Copy link
Contributor

As much as I would prefer to switch to Ember for the 0.19 series, as long as the user experience for Laika's particular use case seems to be superior with blaze I'm reluctant to do so. The issues I observe with Ember are:

  • Delayed shutdown - when killing the laikaPreview task by hitting the enter key, there is a delay of 30 seconds before it actually shuts down. Since it is always fairly exactly that amount of time, I suspect it is some internal timeout in Ember that kills some cleanup task that hangs, but I'm not sure of course. The problem is that for the user it looks as if the shutdown is not working or they might wrongly assume it already completed without any message. This problem occurs with or without using SSE.
  • Exceptions when navigating - when navigating to a different page in the preview site, there are lots of broken pipe exceptions, all occurring within 10-20 seconds after navigating. This error only occurs on pages with activated auto-refresh which is based on SSE, it does not happen on pages without an SSE connection.

On the contrary when using blaze the situation looks somewhat better:

  • Shutdown - happens immediately, but is followed by org.http4s.blaze.server.Http1ServerStage$$anon$1 - Error writing body exceptions a few seconds later.
  • Navigation - happens without any exceptions, both with or without SSE.

Comparing the two scenarios the blaze version appears superior for Laika, despite not being perfect either.

@armanbilge I did not bring this to the http4s repo yet, as I am not sure whether any of the above is an issue in http4s or just me using it wrongly, either in the way how I shut it down or in the way I initiate the SSE connection. I did not find much documentation on the latter, so it's entirely possible I'm not doing it in the intended way (even though it clearly seems to be working otherwise).

@jenshalm jenshalm added this to the 0.19.0 milestone Aug 20, 2022
@armanbilge
Copy link
Member

Hi, thanks for pinging me. Before we revert this can we have some time to investigate and fix the issues?

For example, did you try setting the shutdownTimeout to your preference?
https://github.com/http4s/http4s/blob/77e4f3adc73875ac62ea9def48542bdb58365868/ember-server/shared/src/main/scala/org/http4s/ember/server/EmberServerBuilder.scala#L126

@armanbilge
Copy link
Member

Also, which version of http4s/CE did you experience these issues with? Note that some changes to unhandled error reporting in CE3 caused some noisy logs until they were fixed, which some users were experiencing in http4s/http4s#6219 (comment).

@jenshalm
Copy link
Contributor Author

Thanks for all the pointers, we might be able to close this.

Regarding the graceful shutdown, I somehow did not expect this to be an explicit feature in one backend, but not the other. It seems like the 30 seconds is simply the default in Ember. When I set it to 2 seconds it works as expected and feels like decent UX, unless you think that's dangerously short, but I think for this local, single-user use case it should be fine, right?

Regarding the SSE issue, I tested with latest http4s, but not with latest CE. With the latest CE I cannot reproduce the broken pipe so far (with older CE versions it was reproducible with each page navigations). I'm not closing the EventSource on the client, I would have expected this to happen implicitly when navigating away from a page, or not?

I will do some more testing in the coming days. There is no hurry to merge this anyway, as it can stay open until I do an RC which is weeks away as I still need to do all the planned work on Helium.

@armanbilge
Copy link
Member

When I set it to 2 seconds it works as expected and feels like decent UX, unless you think that's dangerously short, but I think for this local, single-user use case it should be fine, right?

Seems fine to me :)

Regarding the SSE issue, I tested with latest http4s, but not with latest CE. With the latest CE I cannot reproduce the broken pipe so far (with older CE versions it was reproducible with each page navigations).

Right, I suspect this was just an issue of noisy logging. typelevel/cats-effect#2868 added more aggressive logging of unhandled errors which needed some subsequent adjustments.

I'm not closing the EventSource on the client, I would have expected this to happen implicitly when navigating away from a page, or not?

I mean, now we are opening a discussion about browsers 😜 FWIW I wouldn't assume anything /shrug

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

Successfully merging this pull request may close these issues.

2 participants