fix(astro): handle invalid encrypted props in server island#14786
fix(astro): handle invalid encrypted props in server island#14786matthewp merged 7 commits intowithastro:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 5234c2a The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 Changeset Validation Results❌ Changeset validation failed Issues Found:
|
CodSpeed Performance ReportMerging #14786 will not alter performanceComparing Summary
Footnotes |
as per linter analysis
(attempt to)
florian-lefebvre
left a comment
There was a problem hiding this comment.
There will soon be a PR about slots encryption so let's wait until said PR lands so this PR can also tackle error handling of slots decryption. Other than that it looks pretty good thanks!
|
@florian-lefebvre that was here: #14772 already merged and released. If @mef wants to update this PR to do that as well that's fine with me, but also don't think we should block this improvement because of other things that should also be improved. |
|
Thanks, I just implemented the same logic for encrypted slots, and included tests. |
|
Tests are random failures, I'll rerun. |
Changes
p) param of server islands fetching are invalid. This prevents an error from the node:crypto to be logged to the server without any context information. More details in the bug report Server Islands: Decryption errors unhandled with no stack trace #14768.Testing
astro/test/server-islands-test.js:Docs
No change is needed for Astro users.
Before this fix, an error is logged without any context, making it not clear at all that the issue was linked to server islands. After this fix, error 400 explains that
Encrypted props value is invalid.Other
Open to suggestions regarding the label of the badRequest message, and potentially extra logging.