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

Prevent exceptions thrown from 'compatibility mode' routes from being silently caught, disabling compatibility mode #815

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

36degrees
Copy link
Contributor

We automatically ‘enable’ compatibility mode if the file app/v6/routes.js exists.

We currently do this by including it within a try/catch block, relying on the empty catch block handling the case where the file does not exist.

However this means that if any exception is encountered whilst requiring that file, we silently fail and do not enable compatibility mode.

This means that if e.g. a user’s v6 routes file includes a dependency which is not installed (which is likely if the user is trying to update their prototype, as doing so nukes any additional dependencies in their package.json file) then this error will not be reported and compatibility mode just won’t work.

Instead, we test for the presence of app/v6/routes.js – if it exists, then we require it. If any exception is encountered in doing so, it’ll be thrown as expected.

Fixes #799

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-815 October 8, 2019 15:50 Inactive
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

Nice, good to go with a CHANGELOG entry 👍

@NickColley NickColley added this to the Next milestone Oct 8, 2019
@36degrees 36degrees changed the title Improve error handling in compatibility mode Prevent exceptions thrown from 'compatibility mode' routes from being silently caught, disabling compatibility mode Oct 9, 2019
We automatically ‘enable’ compatibility mode if the file app/v6/routes.js exists.

We currently do this by including it within a try/catch block, relying on the empty catch block handling the case where the file does not exist.

However this means that if any exception is encountered whilst requiring that file, we silently fail and do not enable compatibility mode.

This means that if e.g. a user’s v6 routes file includes a dependency which is not installed (which is likely if the user is trying to update their prototype, as doing so nukes any additional dependencies in their package.json file) then this error will not be reported and compatibility mode just won’t work.

Instead, we test for the presence of app/v6/routes.js – if it exists, then we require it. If any exception is encountered in doing so, it’ll be thrown as expected.

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

Successfully merging this pull request may close these issues.

Backwards compatibility mode silently fails to work if there are e.g. unmet dependencies
3 participants