-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: standardise error handling #7185
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
Conversation
🦋 Changeset detectedLatest commit: 9277e31 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
I get this error when running pnpm change: |
I would add a fix for this: #7165, but i have no idea how to fix it. |
@shairez any chance we can get this PR reviewed? |
for the origin/upcoming, try So if I understand correctly, the only somewhat breaking change is that 4xx errors now throw on the client side, right? That seems more like a bugfix than a breaking change. |
Thanks @wmertens ! Yes, the only breaking change is that 4xx errors now throw on the client. It should be classified as a bug fix. I've added a changeset now, I had to temporarily change the changset config |
commit: |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
starters/apps/qwikcity-test/src/routes/(common)/server-func/server-error/index.tsx
Show resolved
Hide resolved
packages/qwik-city/src/middleware/request-handler/user-response.ts
Outdated
Show resolved
Hide resolved
74535d4
to
537576d
Compare
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.
great job Dustin!
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 great, just two small things
packages/docs/src/routes/docs/(qwikcity)/error-handling/index.mdx
Outdated
Show resolved
Hide resolved
packages/qwik-city/src/middleware/request-handler/error-handler.ts
Outdated
Show resolved
Hide resolved
Gentle nudge @wmertens |
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.
@shairez can you merge these changes please |
Thanks again @DustinJSilk ! great job! |
Thanks both! |
What is it?
Description
This fixes a few issues with server$ error handling
Checklist
pnpm change