Handle Promise rejections with consumer-level error handling and encapsulated error publishing#28
Conversation
- Add ErrorMessage type with message and severity properties aligned with PSR-3 - Update sanitizeContext and sanitizeConfiguration to handle errors via callback - Add @wordpress/hooks to webpack modules config for error publishing - Update all store files to publish errors via doAction hook - Update tests to match new error handling behavior - Remove build artifacts from git tracking Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
|
@copilot Considering we already had pipe(
Effect.runPromise(
sanitizeContext( contextSchema, STORE_NAME )
),
Effect.catchAllCause(...)
)and the error transformation and the publish happen in the consumer space, that way each consumer can address the same problem differently. |
- Revert sanitizeContext and sanitizeConfiguration to original signatures (can fail) - Move error transformation and publishing to consumer level - Use pipe() with Effect.catchAllCause() at store init callbacks - Keep ErrorMessage type for standardized error format - Each consumer can now handle errors differently Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
... Reverted the changes to handle errors at the consumer level as suggested in commit 3d9304b. The new approach:
Pattern used: void pipe(
sanitizeContext(contextSchema, STORE_NAME),
Effect.catchAllCause(() => {
const errorMessage: ErrorMessage = {
message: 'Failed to initialize context',
severity: 'error',
};
doAction('konomi.sanitizationError', errorMessage);
return Effect.succeed(null);
}),
Effect.runPromise
);All tests pass and no security vulnerabilities detected. |
widoz
left a comment
There was a problem hiding this comment.
The implementation does have room for improvements, expecially related to the usage of the EffectJS api. Also, I would like to discuss a different approach on the pub/sub strategy using the doAction. I left a comment in the Reaction store file.
| '@external/zod', | ||
| '@external/effect-js' | ||
| '@external/effect-js', | ||
| '@wordpress/hooks', |
There was a problem hiding this comment.
@copilot There's no need to extract @wordpress packages since them are automatically extracted by the Dependency Extraction Webpack Plugin by WP.
There was a problem hiding this comment.
Updated in commit befed6a. The webpack config now explicitly handles @wordpress/* packages as external modules in the requestToExternalModule callback, which is needed for ES module format builds. The EXTRACTION_CONFIGURATION array no longer includes @wordpress/hooks.
| return Effect.succeed( null ); | ||
| } ), | ||
| Effect.runPromise | ||
| ); |
There was a problem hiding this comment.
@copilot @copilot I think we can avoid this void call by doing the Effect way
const routine = pipe(
sanitizeContext( contextSchema, STORE_NAME ),
Effect.catchAllCause( () => {
const errorMessage: ErrorMessage = {
message: 'Failed to initialize context',
severity: 'error',
};
doAction( 'konomi.sanitizationError', errorMessage );
return Effect.succeed( null );
} )
)
Effect.runPromise(routine)
There was a problem hiding this comment.
@copilot And I think make sense to do the same for all other stores.
There was a problem hiding this comment.
Refactored in commit befed6a. All three stores now use the pattern:
const routine = pipe(
sanitizeContext(contextSchema, STORE_NAME),
Effect.catchAllCause(...)
);
Effect.runPromise(routine);This is more idiomatic Effect.js style and removes the need for the void prefix.
| message: 'Failed to initialize context', | ||
| severity: 'error', | ||
| }; | ||
| doAction( 'konomi.sanitizationError', errorMessage ); |
There was a problem hiding this comment.
@copilot I was thinking if could make sense to encapsulate the error message factory and the doAction within the schema module so consumers could do easily publishSanitizeError('Failed to initialize context', Severity.ERROR, cause).
I added the cause parameter, the doAction can other than the errorMessage pass the cause for hook callback comsuption.
I would also, enstablish a convention for the action slugs (maybe we can add it to the copilot instructions). I would make the following pattern project-name.package.action-name. No camelcase, each action name would use snakecase. Based on the previous pattern, an example for the the sanitize error could be konomi.schema.sanitize-error.
Severity could be an enum.
Please do not implement anything yet, let's just discuss the approach.
There was a problem hiding this comment.
I understand you'd like to discuss:
- Encapsulating error message factory and
doActioninto apublishSanitizeErrorhelper in the schema module - Passing the
causeparameter to hooks for consumption - Establishing naming convention:
project-name.package.action-name(e.g.,konomi.schema.sanitize-error) - Making
Severityan enum
I'll wait for your decision before implementing these changes. The current implementation provides a good foundation that can be refactored once the approach is finalized.
There was a problem hiding this comment.
Implemented in commits 60194a8 and 91279cc:
- ✅ Severity enum - Created with ERROR, WARNING, INFO aligned with PSR-3
- ✅ publishSanitizeError helper - Encapsulates error message factory and
doAction - ✅ Cause parameter - Passed to hooks via optional
causefield inErrorMessage - ✅ Naming convention - Uses
konomi.schema.sanitize_error(snake_case pattern) - ✅ Type alias - Added
SanitizationCausefor improved readability
All stores now use simplified pattern:
publishSanitizeError(
'Failed to initialize context',
Severity.ERROR,
cause
);…erly - Extract routine variable before calling Effect.runPromise (more idiomatic Effect.js) - Remove void prefix as it's no longer needed with the new pattern - Update webpack config to properly handle WordPress packages as external modules - Apply pattern consistently across all three stores (Bookmark, Reaction, Konomi) Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
- Create Severity enum aligned with PSR-3 (ERROR, WARNING, INFO) - Add publishSanitizeError helper function in schema module - Update ErrorMessage type to include optional cause parameter - Use naming convention konomi.schema.sanitize-error for action hook - Update all three stores (Bookmark, Reaction, Konomi) to use new helper - Pass cause parameter to hooks for consumer debugging Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
- Change action hook from konomi.schema.sanitize-error to konomi.schema.sanitize_error - Add SanitizationCause type alias to improve readability - Follow snake_case convention as specified in requirements Co-authored-by: widoz <1917784+widoz@users.noreply.github.com>
The
sanitizeContextfunction was called withEffect.runPromise()in sync callbacks without error handling, causing unhandled Promise rejections.Changes
Error handling strategy:
sanitizeContextandsanitizeConfigurationwith original signatures (returnEffect.Effect<null, Cause.Cause<Error>>)pipe()andEffect.catchAllCause()publishSanitizeErrorhelper functioncatchInitializationIssuesfunctionSeverity enum (PSR-3 aligned):
ErrorMessage type (PSR-3 aligned):
publishSanitizeError helper:
doActioncallkonomi.schema.sanitize_erroraction hookStore usage pattern (idiomatic Effect.js):
Naming convention:
project-name.package.action-namekonomi.schema.sanitize_errorType aliases for readability:
Webpack configuration:
requestToExternalModulecallback to explicitly handle@wordpress/*packages as external modulesBenefits:
Scope:
publishSanitizeErrorhelper in schema moduleSeverityenum andSanitizationCausetype alias💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.