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

Client <-> Server version handling (Version Mismatch Errors) #6949

Closed
3 tasks done
ErisDS opened this issue Jun 9, 2016 · 2 comments
Closed
3 tasks done

Client <-> Server version handling (Version Mismatch Errors) #6949

ErisDS opened this issue Jun 9, 2016 · 2 comments
Assignees
Labels
affects:admin Anything relating to Ghost Admin server / core Issues relating to the server or core of Ghost
Milestone

Comments

@ErisDS
Copy link
Member

ErisDS commented Jun 9, 2016

As Ghost grows up, and we're doing bigger migrations and other changes between versions, we're starting to feel the need to be a bit smarter about updating, particularly around the client - server split.

We need a way for a client (particularly the admin client) to know when the server side is running a version ahead of it, so that it can prompt users that they need to refresh. This is particularly important when the data model changes, to prevent the client sending invalid data to the server.

This needs to be implemented in a robust way, that lets us handle API versioning in a better way in future and that will also work for clients other than the Ghost Admin.

As of TryGhost/Admin#38, the admin client sends an X-Ghost-Version header with every request to the API. This sends the first two digits of the version number (e.g. 0.8) to the server, to say "this is the version I'm expecting you to be right now".

On the server side, we need to add some middleware to receive that header, and return a brand new "400 Version Mismatch Error" if, for example, the server is running 0.9. This is not as straightforward as it sounds, because our error handling for the API doesn't use middleware properly and so the error doesn't get correctly sent as JSON... I have a WIP PR for the middleware, but the error refactor should probably land first.

Once we are correctly sending Version Mismatch Errors from the server, we need to update Ghost-Admin to handle those errors, and show the user a "Please refresh to update" message. Due to the use of minor-version numbers, this message will only be appearing for minor updates e.g. 0.8 -> 0.9, not patch updates like 0.9.0 -> 0.9.1. Therefore, if we want to change bits of the data model, we really need to do minor version bumps - we've always done this in the past, but it has now become more important.

Version Matching Tasks:

@ErisDS ErisDS added affects:admin Anything relating to Ghost Admin server / core Issues relating to the server or core of Ghost labels Jun 9, 2016
@ErisDS
Copy link
Member Author

ErisDS commented Jun 9, 2016

Ok - the error handling issue here was a false alert - I'd just wired my middleware up incorrectly 😞

I had totally missed/forgotten the fact that we do have error handling middleware on the API

, and thought we were solely relying on the catch here:
}).catch(function onAPIError(error) {

As we do in fact have error handling middleware in place correctly, I just needed to move my wiring for the middleware from the blogApp to the API router, and bobs your uncle it all works.

PR INCOMING

ErisDS added a commit to ErisDS/Ghost that referenced this issue Jun 9, 2016
refs TryGhost#6949

- Adds a new VersionMismatchError with status 400 (bad request)
- Adds middleware that checks the X-Ghost-Version header if it is provided
- If it is not provided, the middleware does nothing
- If it is provided, and the versions match, the middleware does nothing
- If it is provided, and the versions don't match, the middleware returns a VersionMismatchError
- Includes both unit and a functional test to prove the middleware works alone and as part of the whole system
@kevinansfield kevinansfield added this to the 0.9.0 milestone Jun 20, 2016
@kevinansfield kevinansfield self-assigned this Jun 20, 2016
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this issue Jul 6, 2016
refs TryGhost/Ghost#6949

Handle version mismatch errors by:
- displaying an alert asking the user to copy any data and refresh
- disabling navigation so that unsaved data is not accidentally lost

Detailed changes:
- add `error` action to application route for global route-based error handling
- remove 404-handler mixin, move logic into app route error handler
- update `.catch` in validation-engine so that promises are rejected with the
  original error objects
- add `VersionMismatchError` and `isVersionMismatchError` to ajax service
- add `upgrade-status` service
  - has a method to trigger the alert and toggle the "upgrade required" mode
  - is injected into all routes by default so that it can be checked before
    transitioning
- add `Route` override
  - updates the `willTransition` hook to check the `upgrade-status` service
    and abort the transition if we're in "upgrade required" mode
- update notifications `showAPIError` method to handle version mismatch errors
- update any areas where we were catching ajax errors manually so that the
  version mismatch error handling is obeyed
- fix redirect tests in editor acceptance test
- fix mirage's handling of 404s for unknown posts in get post requests
- adjust alert z-index to to appear above modal backgrounds
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this issue Jul 7, 2016
refs TryGhost/Ghost#6949

Handle version mismatch errors by:
- displaying an alert asking the user to copy any data and refresh
- disabling navigation so that unsaved data is not accidentally lost

Detailed changes:
- add `error` action to application route for global route-based error handling
- remove 404-handler mixin, move logic into app route error handler
- update `.catch` in validation-engine so that promises are rejected with the
  original error objects
- add `VersionMismatchError` and `isVersionMismatchError` to ajax service
- add `upgrade-status` service
  - has a method to trigger the alert and toggle the "upgrade required" mode
  - is injected into all routes by default so that it can be checked before
    transitioning
- add `Route` override
  - updates the `willTransition` hook to check the `upgrade-status` service
    and abort the transition if we're in "upgrade required" mode
- update notifications `showAPIError` method to handle version mismatch errors
- update any areas where we were catching ajax errors manually so that the
  version mismatch error handling is obeyed
- fix redirect tests in editor acceptance test
- fix mirage's handling of 404s for unknown posts in get post requests
- adjust alert z-index to to appear above modal backgrounds
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this issue Jul 8, 2016
refs TryGhost/Ghost#6949

Handle version mismatch errors by:
- displaying an alert asking the user to copy any data and refresh
- disabling navigation so that unsaved data is not accidentally lost

Detailed changes:
- add `error` action to application route for global route-based error handling
- remove 404-handler mixin, move logic into app route error handler
- update `.catch` in validation-engine so that promises are rejected with the
  original error objects
- add `VersionMismatchError` and `isVersionMismatchError` to ajax service
- add `upgrade-status` service
  - has a method to trigger the alert and toggle the "upgrade required" mode
  - is injected into all routes by default so that it can be checked before
    transitioning
- add `Route` override
  - updates the `willTransition` hook to check the `upgrade-status` service
    and abort the transition if we're in "upgrade required" mode
- update notifications `showAPIError` method to handle version mismatch errors
- update any areas where we were catching ajax errors manually so that the
  version mismatch error handling is obeyed
- fix redirect tests in editor acceptance test
- fix mirage's handling of 404s for unknown posts in get post requests
- adjust alert z-index to to appear above modal backgrounds
@acburdine
Copy link
Member

Now that the client PR was merged in I think this can be closed 😄

geekhuyang pushed a commit to geekhuyang/Ghost that referenced this issue Nov 20, 2016
refs TryGhost#6949

- Adds a new VersionMismatchError with status 400 (bad request)
- Adds middleware that checks the X-Ghost-Version header if it is provided
- If it is not provided, the middleware does nothing
- If it is provided, and the versions match, the middleware does nothing
- If it is provided, and the versions don't match, the middleware returns a VersionMismatchError
- Includes both unit and a functional test to prove the middleware works alone and as part of the whole system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:admin Anything relating to Ghost Admin server / core Issues relating to the server or core of Ghost
Projects
None yet
Development

No branches or pull requests

3 participants