-
Notifications
You must be signed in to change notification settings - Fork 426
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(migrate): fixes error message to be more helpful #6986
fix(migrate): fixes error message to be more helpful #6986
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
No changes to documentation |
8bf33d5
to
c127717
Compare
Component Testing Report Updated Jun 18, 2024 7:08 PM (UTC)
|
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.
had a few ✨ suggestions but this is already better so ✅
const mockResponse = { | ||
status: 500, | ||
statusText: 'Internal Server Error', | ||
json: () => | ||
Promise.resolve({ | ||
error: { | ||
type: 'validationError', | ||
description: 'Document is not of valid type', | ||
}, | ||
status: 500, | ||
}), | ||
} |
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.
If our env supports the response constructor, this may be nicer
const mockResponse = { | |
status: 500, | |
statusText: 'Internal Server Error', | |
json: () => | |
Promise.resolve({ | |
error: { | |
type: 'validationError', | |
description: 'Document is not of valid type', | |
}, | |
status: 500, | |
}), | |
} | |
const mockResponse = new Response( | |
JSON.stringify({ | |
error: { | |
type: 'validationError', | |
description: 'Document is not of valid type', | |
}, | |
status: 500, | |
}), | |
{ | |
status: 500, | |
statusText: 'Internal Server Error', | |
}, | |
) |
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.
probably worth doing it but maybe in a follow up PR, would need to change a few tests
Description
Seems like the error response from backend is in the shape below
When we receive error in this shape we don't properly log that message making it hard for folks to debug. See screenshots below
Before
After
What to review
Copy and change makes sense, I checked the repo and there seems to be other places checking
error.description
so I am not sure if error will not be an object but I added a conditional to keep backwards compatibilityTesting
Updated the test
Notes for release