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

fix: update createError parameter signature to match h3 version in Nuxt. #27418

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Ray0907
Copy link

@Ray0907 Ray0907 commented Jun 1, 2024

🔗 Linked issue

#28632

📚 Description

This pull request updates the createError parameter signature to align with the h3 version used in Nuxt. The previous signature did not match, which could cause inconsistencies and potential issues when creating errors. This change ensures compatibility and improves the robustness of error handling.
ref: https://h3.unjs.io/guide/event-handler#error-handling

Copy link

stackblitz bot commented Jun 1, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@danielroe
Copy link
Member

danielroe commented Jun 6, 2024

statusCode is used consistently in Nitro, e.g.:

https://github.com/unjs/nitro/blob/d20ffcbd16fc4003b774445e1a01e698c2bb078a/src/runtime/internal/task.ts#L37-L49

The move from statusMessage -> message looks good.

What do you think @pi0?

@pi0
Copy link
Member

pi0 commented Jun 7, 2024

If i'm not wrong statusMessage was Node.js flavor.

status and statusText are Web Response props. i would try to adopt them consistently everywhere.

~> unjs/h3#767

@danielroe
Copy link
Member

Contra my previous message, we should not update statusMessage -> message. It should be statusMessage -> statusText. message is a separate field that governs the content of the body.

I'm a little reluctant to update the docs yet as the type of the error returned from createError still has statusCode and statusMessage fields, which is possibly just confusing for people. (For example, an error thrown with { status: 400 } would have { statusCode: 400 } and no status property.)

statusCode and status would be used in different places. (Still required by Nitro render response, for example.) And the error object provided by Nuxt in error.vue and elsewhere still expects those properties. Likely we'd need to implement backwards compatibility with a getter in h3/nuxt.

As a result, I think we need to defer this PR for now. Depending on what we do in h3, we might still merge it in time for 4.x. What do you think @pi0?

@danielroe danielroe marked this pull request as draft June 7, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants