Skip to content

feat(mbe): throw valid error when server is down#105

Merged
pranavjain97 merged 4 commits into
masterfrom
WP-5513/throw-valid-error
Aug 8, 2025
Merged

feat(mbe): throw valid error when server is down#105
pranavjain97 merged 4 commits into
masterfrom
WP-5513/throw-valid-error

Conversation

@cpatino-intive

Copy link
Copy Markdown
Contributor

TASKS: WP-5513

@pranavjain97 pranavjain97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you try calling this.checkServerRunning(error); at the responseHandler instead? That way u dont need to add it for every catch statement.

Comment thread src/kms/kmsClient.ts Outdated
} catch (error: any) {
this.errorHandler(error, 'Error posting key to KMS');

throw new Error(`Failed to post key to KMS: ${error.message}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isnt the error already being thrown by this.errorHandler

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. sorry, there was a conflict when I rebase.

Comment thread src/kms/kmsClient.ts Outdated
} catch (error: any) {
this.errorHandler(error, 'Error decrypting data key from KMS');

throw new Error(`Failed to decrypt data key from KMS: ${error.message}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove all of these new errors, already handled by errorHandler

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@cpatino-intive cpatino-intive force-pushed the WP-5513/throw-valid-error branch from 56bf470 to 3041201 Compare August 7, 2025 13:13
logger.error(
'Failed to create independent keychain: %s',
(error as DecodeError).decodedResponse.body,
(error as DecodeError).decodedResponse?.body,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why does it need be optional now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested it and found that decodedResponse is undefined when the kms or aws is offline and it throws another error because of getting body from decodedResponse

pranavjain97
pranavjain97 previously approved these changes Aug 7, 2025
@pranavjain97 pranavjain97 merged commit 72a4d2a into master Aug 8, 2025
4 checks passed
@pranavjain97 pranavjain97 deleted the WP-5513/throw-valid-error branch August 8, 2025 16:08
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