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

[wip] Improve error page #10505

Closed
wants to merge 1 commit into from
Closed

Conversation

klaussner
Copy link
Contributor

@klaussner klaussner commented Mar 21, 2019

This PR improves how the Meteor Tool handles server crashes and build failures:

  • If an error occurs, clients automatically load the error page.
  • No more Uncaught SyntaxError: Unexpected token < in JSON at position 1 in the console!
  • Clients automatically reload the page after rebuilds.
  • Code frames are displayed with colors instead of ANSI escape codes.
  • The error page works better on mobile devices.

Video comparison of the current and the new version

Technical Details

The autoupdate package, which is responsible for notifying clients of changes, is used in both development and production. Therefore, it's part of the app and not the build tool. However, since the app server is terminated in case of a crash or build failure, autoupdate cannot be used to reload the error page (see the following diagram).

diagram

To solve this problem, the proxy opens an SSE endpoint (/__meteor__/build-events) that the app client and error page use to get notified of build events, which means that there are now two reload mechanisms: one for errors (via SSE) and one for regular rebuilds (via WebSocket/DDP). However, it might make sense to use SSE for regular rebuilds as well in the future.

Future Work

Apps that need autoupdate/hot-code-push only in development (some Apollo apps, for example) still have these packages and large dependencies (including socket-stream-client and ddp-client) in their production bundles. This results in up to 90 kB of unused code as well as an unused WebSocket for each client. I've already reduced the size of autoupdate in #10238 by removing the mongo dependency but that's still a lot of overhead.

However, I think it's possible to support both use cases in a backwards-compatible way. If autoupdate is included in the app, the Meteor Tool can use the current WebSocket/DDP-based reload for regular rebuilds. If the package is not included, it can let the proxy send a reload message to clients via SSE. That way, apps that use autoupdate work exactly as they do now, whereas more lightweight apps don't need any additional packages for development-only reloads.

The app client (via the new `build-events` package) and error page connect to the SSE endpoint `/__meteor__/build-events` provided by the proxy to get notified of server crashes and build failures.
@@ -0,0 +1 @@
../../../packages/build-events/event-handler.js
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't think of another way to share code between the tool and the build-events package… 🤔

@@ -54,7 +54,8 @@ var packageJson = {
multipipe: "2.0.1",
pathwatcher: "7.1.1",
optimism: "0.6.3",
'lru-cache': '4.1.3'
'lru-cache': '4.1.3',
anser: "1.4.8"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires a new dev bundle.

@klaussner klaussner changed the title Improve error page [wip] Improve error page Mar 21, 2019
@benjamn benjamn added this to the Release 1.9.0 milestone Apr 2, 2019
@klaussner
Copy link
Contributor Author

As I mentioned above, automatically reloading the error page requires a new communication channel (WebSocket or server-sent events) between build tool and client and I think it makes sense to move the existing reload mechanism to this new channel as well (otherwise, we would always need to open two channels). See this feature request for more details: meteor/meteor-feature-requests#354. Auto-reloading on the error page can implemented on top of that. 🙂

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

Successfully merging this pull request may close these issues.

None yet

2 participants