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

Issue with 1.18.0-RC1 Cljs build #429

Closed
danielsz opened this issue Jun 26, 2023 · 8 comments
Closed

Issue with 1.18.0-RC1 Cljs build #429

danielsz opened this issue Jun 26, 2023 · 8 comments
Assignees
Labels

Comments

@danielsz
Copy link
Collaborator

danielsz commented Jun 26, 2023

Hi,

I've been using Sente 1.17 stable for years, am now trying to upgrade to 1.18.0-RC1. I am very much aware of the breaking changes, but they don't seem to be affecting my use case. I have no issues starting the server side of Sente, however the client side doesn't load in the browser due to logging output. Here are the first lines of the compiled sente file.

// Compiled by ClojureScript 1.11.60 {:optimizations :none}
Loading initial Timbre config from: :default
goog.provide('taoensso.sente');
goog.require('cljs.core');
goog.require('clojure.string');
goog.require('cljs.core.async');
goog.require('taoensso.encore');
goog.require('taoensso.timbre');
goog.require('taoensso.sente.interfaces')

As can be seen, the second line is not commented out and causes a reference error.

Uncaught SyntaxError: Unexpected identifier 'initial'

When using Sente 1.17, I was not touching the timbre logging system at all and everything worked without configuring anything. Has that change in Sente 1.18?

@ptaoussanis
Copy link
Member

@danielsz Hi Daniel, it's good to hear from you!

Thanks a lot for pinging about this, and for the clear report. Apologies for the trouble!

It looks like you're being affected by this.

Would you please try again, after placing [com.taoensso/timbre "6.2.0-alpha1"] before Sente in your Leiningen/etc. dependencies?

I'll aim to cut new stable Timbre and Sente releases that include this fix before the end of the month.

@danielsz
Copy link
Collaborator Author

Hi Peter, it's good to meet you again! 😃
What you suggested I do is actually something I figured out. I am using [com.taoensso/timbre "6.2.0-alpha1"].
The difference with[com.taoensso/timbre "6.1.0"]is that it got rid of the occurrence of the offending log output in timbre.js but it is also and still present in sente.js.

@ptaoussanis
Copy link
Member

Just to clarify:

  • You've confirmed that you're for sure using [com.taoensso/timbre "6.2.0-alpha1"] (via lein deps :tree or equivalent)
  • You've cleared cl/js build artifacts in your project (via lein clean or equivalent)
  • When you do a fresh build, you're still seeing "Loading initial Timbre config from: :default" in taoensso.sente build artifact.

Is that all correct?

@danielsz
Copy link
Collaborator Author

danielsz commented Jun 26, 2023

Absolutely. That is exactly right. I've done exactly what you wanted to clarify.
I will say: it is late and I've been at it for a while, so it might be wise for me to retry tomorrow morning and confirm again.

@danielsz
Copy link
Collaborator Author

danielsz commented Jun 26, 2023

Oh the wonders of a good night sleep.
Cleaning the build was not enough, I had to switch a compiler option, namely setting :aot-cache to false. Clojurescript does aggressive caching otherwise. The documentation says:

Controls whether the shared AOT cache is used for compiler artifacts produced from JARs.

So once Sente is compiled, it won't be recompiled, no matter that its dependencies ([com.taoensso/timbre "6.2.0-alpha1"]) may have changed.

Anyway, I am happy to report that everything is fine now. Thank you, Peter!

@ptaoussanis
Copy link
Member

Thanks for the update Daniel, happy to hear that you found a solution!

Again, apologies for the trouble on this. I'll have updated Timbre & Sente stable releases up later this week.

Will keep this issue open till then.

@ptaoussanis ptaoussanis changed the title Trying to upgrade to 1.18.0-RC1 Issue with 1.18.0-RC1 Cljs build Jun 27, 2023
@danielsz
Copy link
Collaborator Author

Thank you so much, Peter. I am very grateful.

@ptaoussanis
Copy link
Member

Pushing update in a moment, closing 👍

ptaoussanis added a commit that referenced this issue Jul 12, 2023
…ids`

Big thanks to @krajj7 for the report and huge assistance debugging!

As part of the investigation into this issue, and due to another recent
related issue (#429), I decided that we were overdue for a refactor
of Sente's connection management system.

The old system had grown overly complex, and left too much room for
edge cases and timing issues.

This commit introduces a major refactor of the system, with an
emphasis on robustness and improved observability.

Specific improvements include:

  - New internal "conn-id" concept that:
    1. No longer depends on http server implementations to
       properly implement identity
    2. Greatly improves logging output, easing debugging

  - General logging improvements to ease debugging
  - Now expose internal `conns_` state to ease debugging

  - Added server-side ping timeout to match client side,
    and to catch unexpected cases where http server is
    never able to identify a connection as broken.

    Note that this new feature is currently opt-in[1], but
    will be enabled by default in a future release.

  - Simplified internal API for updating `conns_` state.

  - More robust handling when events fire in unexpected
    order (e.g. :on-close firing before :on-open handshake).

  - Generally improved clarity and robustness.

Note: some related additions will also be made to the
reference example project in another commit.

[1] Provide a value (e.g. 5000) for the new `:ws-ping-timeout-ms`
    option to `make-channel-socket-server!`
ptaoussanis added a commit that referenced this issue Jul 13, 2023
…ids`

Big thanks to @krajj7 for the report and huge assistance debugging!

As part of the investigation into this issue, and due to another recent
related issue (#429), I decided that we were overdue for a refactor
of Sente's connection management system.

The old system had grown overly complex, and left too much room for
edge cases and timing issues.

This commit introduces a major refactor of the system, with an
emphasis on robustness and improved observability.

Specific improvements include:

  - New internal "conn-id" concept that:
    1. No longer depends on http server implementations to
       properly implement identity
    2. Greatly improves logging output, easing debugging

  - General logging improvements to ease debugging
  - Now expose internal `conns_` state to ease debugging

  - Added server-side ping timeout to match client side,
    and to catch unexpected cases where http server is
    never able to identify a connection as broken.

    Note that this new feature is currently opt-in[1], but
    will be enabled by default in a future release.

  - Simplified internal API for updating `conns_` state.

  - More robust handling when events fire in unexpected
    order (e.g. :on-close firing before :on-open handshake).

  - Generally improved clarity and robustness.

Note: some related additions will also be made to the
reference example project in another commit.

[1] Provide a value (e.g. 5000) for the new `:ws-ping-timeout-ms`
    option to `make-channel-socket-server!`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants