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

Sandbox error handling updates #990

Merged
merged 6 commits into from
Feb 5, 2024
Merged

Conversation

adamawang
Copy link
Contributor

@adamawang adamawang commented Jan 31, 2024

Description and Context

For context, a migration was made to use local-dev-lib from cli-lib and some of the error message formats have changed. The util functions we are using on hubspot-cli (eg isSpecifiedError, isMissingScopeError) were outdated and were expecting a different error format.

This PR aims to fix error handling for hs sandbox commands by migrating to the local-dev-lib versions of the util functions. I've also added a missing error scenario for hs sandbox create where users may not be ungated for development sandboxes.

Required changes in local-dev-lib: HubSpot/hubspot-local-dev-lib#93

Screenshots

Before 1:
before1

After 1:
after1

Before 2:
before2

After 2:
after2

TODO

NOTE: I wasn't able to migrate isSpecifiedError to the local-dev-lib version for cli/lib/projects.js and cli/commands/project/dev.js. The APIs being called in those files still come from cli-lib and use request-promise-native so the new LDL version of isSpecifiedError wont work since they're not AxiosErrors.

Who to Notify

@shannon-lichtenwalter

@adamawang adamawang self-assigned this Jan 31, 2024
Comment on lines +66 to +68
const {
isMissingScopeError,
} = require('@hubspot/local-dev-lib/errors/apiErrors');
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 migrated over isMissingScopeError to the LDL version since its for a sandboxes endpoint, but left isSpecifiedError as the local version. The usage of isSpecifiedError in this file is for handleProjectUpload which is still pulling uploadProject from the old @hubspot/cli-lib dependency where the http base is still request-promise-native.

Same goes for cli -> lib -> projects.js in this repo since fetchProject is also pulled in from @hubspot/cli-lib. Usage is limited to just those two in projects files - I'll leave a comment in the code to reduce any confusion once that migration happens

Comment on lines +105 to +108
isSpecifiedError(err, {
statusCode: 403,
category: 'BANNED',
subCategory: 'SandboxErrors.DEVELOPMENT_SANDBOX_ACCESS_NOT_ALLOWED',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a gating error scenario for when users are not ungated for crm-developement-beta

Copy link
Contributor

@brandenrodgers brandenrodgers left a comment

Choose a reason for hiding this comment

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

🚢

@adamawang adamawang merged commit e094a05 into master Feb 5, 2024
1 check passed
@adamawang adamawang deleted the fix-errors-for-sandbox-commands branch February 5, 2024 22:54
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