-
Notifications
You must be signed in to change notification settings - Fork 0
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
mrc-6085 Error handling #3
Merged
Merged
Changes from 12 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
0d14148
first stab at error handler
EmmaLRussell 4d98f83
use async controller handler
EmmaLRussell dd25f57
comments
EmmaLRussell 5894fb9
return json for all 404s
EmmaLRussell 3bcbe9a
return 400 on bad z, x or y
EmmaLRussell 0310015
use jsonResponseSuccess for index response
EmmaLRussell 381611c
unit tests for controller and controller handler
EmmaLRussell dfdf1ec
lint
EmmaLRussell f6a83f7
more tests
EmmaLRussell 2d5c3c0
added error testing endpoint for running in testing modes
EmmaLRussell ab485a9
error endpoint test, lint, grey country border for test page
EmmaLRussell 507c75d
merge serve tile changes
EmmaLRussell 9224ee0
Remove status param from GroutError constructor
EmmaLRussell bbb2e95
fix requestWithError mess
EmmaLRussell 326efdc
try git_sha
EmmaLRussell 4ad0ca4
fix script
EmmaLRussell cecd991
set pr sh from context
EmmaLRussell e278c21
make GroutError status a getter
EmmaLRussell aabe83c
try to make test page not look like it's trying to be a thing
EmmaLRussell 3ea2a0e
use RequestWithError as handleError param type
EmmaLRussell fef5ebb
lint
EmmaLRussell e8491fd
Merge branch 'main' into mrc-6085-error-handling
EmmaLRussell bc88234
merge with main
EmmaLRussell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have expected a docker/run script to be used for a deployment. Maybe the env var check in src/routes.ts should also check what environment we're in directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is just a dev/CI script, we won't be using it for deployment, that'll be a separate tool. I think it's easier to be explicit when we want the testing route to be added (really just when using this script for integration testing locally/on CI).