Small improvements to error handling #760
Conversation
src/deploy.ts
Outdated
| if (isHttpError(error) && error.statusCode === 404) { | ||
| if (isHttpError(error) && error.statusCode === HttpStatusCode.NOT_FOUND) { |
There was a problem hiding this comment.
I'm not convinced about this indirection for http status codes. It's as easy to look up (or remember) numbers, esp. given that the error is rephrased just below.
There was a problem hiding this comment.
@Fil this change definitely doesn't need to be added. I just think it's easier to think about the status message --> the error message instead of status code --> status message --> error message
There was a problem hiding this comment.
I'd prefer the minimal diff. But at least this helps to show that we're using unauthenticated in this code in lieu of UNAUTHORIZED, which doesn't have the same meaning (in the sense that you could be authenticated as user x but not authorized to do action z).
There was a problem hiding this comment.
Tested by attempting to upload a huge file (1GB):
└ Error: File too large to deploy: docs/hugefile.mp4. Maximum file size is 50MB
so that's good 👍
But it's surprising to me that we did not detect the issue before that 1GB was (completely) sent over to the server. Maybe in a follow-up we could anticipate and show the error much earlier?
|
@Fil yep! we're working on adding a preflight check |
Handles a couple of cases: