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

fix: maintenance mode should assume disable if db call fails. #6120

Merged
merged 2 commits into from
Feb 3, 2024

Conversation

ivarconr
Copy link
Member

@ivarconr ivarconr commented Feb 3, 2024

Usually maintenance mode is disabled. If the call throws, which we see a lot of when a unleash instance is in terminating state, we should return a default value.

By having it throw inside of the memoizee function, the response is not cached, and it will trigger new calls until it return a cachable result.

Copy link

vercel bot commented Feb 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Feb 3, 2024 7:47am
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Feb 3, 2024 7:47am

Usually maintenance mode is disabled. If the call throws, which we
see a lot of when a unleash instance is in terminating state, we
should return a defualt value.

By having it throw inside of the memoizee function, the response is
not cached, and it will trigger new calls until it return a cachable
result.
@ivarconr ivarconr force-pushed the fix/maintenance-mode-handle-db-errors-with-default branch from dcb50e0 to f1bbef0 Compare February 3, 2024 07:45
try {
return (
this.config.flagResolver.isEnabled('maintenanceMode') ||
(await this.resolveMaintenance())
Copy link
Member Author

Choose a reason for hiding this comment

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

by allowing the momizee function to throw, we unsure it will not be cached.

Copy link
Contributor

@gastonfournier gastonfournier left a comment

Choose a reason for hiding this comment

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

LG!

@@ -193,7 +193,7 @@ export default class VersionService {
)) ?? { id: undefined };
this.instanceId = id;
} catch (err) {
this.logger.warn('Could not find instanceInfo');
this.logger.warn('Could not find instanceInfo', err);
Copy link
Contributor

Choose a reason for hiding this comment

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

🥇

@ivarconr ivarconr merged commit 77b7cb0 into main Feb 3, 2024
6 checks passed
@ivarconr ivarconr deleted the fix/maintenance-mode-handle-db-errors-with-default branch February 3, 2024 08:17
gastonfournier pushed a commit that referenced this pull request Feb 5, 2024
Usually maintenance mode is disabled. If the call throws, which we see a
lot of when a unleash instance is in terminating state, we should return
a default value.

By having it throw inside of the memoizee function, the response is not
cached, and it will trigger new calls until it return a cachable result.
gastonfournier added a commit that referenced this pull request Feb 5, 2024
#6118 
#6119 
#6120
#6124 
#6125

---------

Co-authored-by: Ivar Conradi Østhus <[email protected]>
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.

2 participants