-
Notifications
You must be signed in to change notification settings - Fork 8
Fix(attachments): Improve Error Handling in Upload Lifecycle and Non-draft Upload Flow #228
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
base: main
Are you sure you want to change the base?
Conversation
…t operations Wrapped nonDraftHandler(), put(), and updateContentHandler() in try/catch blocks to ensure proper error propagation using req.error(...). • Ensured await super.put(...) is followed by error-safe logic in put(). • Invoked scanRequest() asynchronously with catch() to avoid crashing on malware scanner failures. • Prevented unhandled exceptions during file uploads and metadata updates in non-draft flow.
…istent CAP logging
…raftUpload - Wrap srv.before("NEW"/"CREATE") handler with try/catch to log errors and reject on failure during attachment initialization - Update nonDraftUpload function to handle errors gracefully, log failures, and reject to prevent unhandled exceptions Improves robustness by preventing crashes due to unhandled errors in attachment upload lifecycle.
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.
Looks good. Overall I have 2 questions on the code.
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.
How the non-draft and draft scenarios be distinguishable from the usage perspective?
try { | ||
if (req?.content?.url?.endsWith("/content")) { | ||
const attachmentID = req.content.url.match(attachmentIDRegex)[1]; | ||
await AttachmentsSrv.nonDraftHandler(target, { ID: attachmentID, content: req.content }); |
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.
Doesn't the line 117 need to be executed only after the attachmentID is obtained ? I mean would there be a situation when AttachmentsSrv.nonDraftHandler is called when url.match is asynchronously being executed ?
This PR addresses issue #227 with better error handling, especially for non-draft attachment uploads:
• Added robust error handling and logging in attachment creation and upload handlers.
• Improved validation and rejection of invalid requests to prevent unhandled exceptions.
• Enhanced overall stability to avoid server crashes during file upload operations.
Also the documentation update is also covered as part of this PR
These changes improve reliability and make debugging easier.