qol(endpoints): helpful error message when a response is not provided #10455
Conversation
🦋 Changeset detectedLatest commit: bf2d6b7 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 |
github-actions
bot
added
pkg: astro
|
|
||
| if (!response || response instanceof Response === false) { | ||
| throw new AstroError(EndpointDidNotReturnAResponse) | ||
| } |
There was a problem hiding this comment.
Isn't this handled by the renderPage function of the pipeline? If not, then a test should be cover this edge case
There was a problem hiding this comment.
The renderPage function wouldn't run for endpoints. I'm guessing you meant RenderContext.render() - it checks the return of the middleware. Failures from the absence of a response occur before control flow returns to it.
An error here is not new, it was just thrown by accessing response.status before, now it thrown explicitly. A test would work the same before and after this PR.
There was a problem hiding this comment.
A test would work the same before and after this PR.
Now we throw an astro error, we can assert that the correct error is thrown
There was a problem hiding this comment.
A different new error would be logged, but the tests will have access to a 500 response either way.
There was a problem hiding this comment.
Still, I believe it's worth expanding our test suite in this case. Any throw could return a 500, although this is a specific error that Astro decides to handle in a meaningful way with a proper message.
Such cases should to be tested. We do this in our middleware, I don't see any reason why we shouldn't do the same here.
There was a problem hiding this comment.
Okay, that makes sense. I had thought you meant a regression test.
Changes
cant read status of undefined(we do a check onresponse.status)Testing
Docs