Conversation
|
That's a shiny PR |
cubap
left a comment
cubap
left a comment
There was a problem hiding this comment.
I think I'm okay with this. I didn't break it. I have some comments, but the most general one is that this is only TinyThings and not TinyPEN. If that is good with you - send it.
There was a problem hiding this comment.
Technically as middle ware I think the rerum response would come last but I see you
| rerum_err_text = undefined | ||
| } | ||
| if (rerum_err_text) error.message = rerum_err_text | ||
| else { |
There was a problem hiding this comment.
You know I would have put a ternary because I hate "else" but it is kinda an else anyway.
However, no braces and then braces for this block pair was a bold move.
There was a problem hiding this comment.
oh yeah i might have had more than one line in there at one point
| // Perhaps this is a more generic 500 from the app, perhaps involving RERUM, and there is no good rerum_error_res | ||
| error.message = rerum_error_res.statusMessage ?? rerum_error_res.message ?? `A server error has occured` | ||
| } | ||
| error.status = rerum_error_res.statusCode ?? rerum_error_res.status ?? 500 |
| error.status = rerum_error_res.statusCode ?? rerum_error_res.status ?? 500 | ||
| console.error(error) | ||
| res.set("Content-Type", "text/plain; charset=utf-8") | ||
| res.status(error.status).send(error.message) |
There was a problem hiding this comment.
This means messenger can never have a payload...
There was a problem hiding this comment.
For now. We didn't respond anywhere with a payload. If we end up wanting to do that we can upgrade the messenger.
| // Special handling. This does not go through to error-messenger.js | ||
| if (rerum_res.status === 409) { | ||
| const currentVersion = await rerum_res.json() | ||
| return res.status(409).json(currentVersion) |
There was a problem hiding this comment.
This sends it, it doesn't keep going.
| if(res.headersSent) return | ||
| const result = await response.json() | ||
| // Send RERUM error responses to error-messenger.js | ||
| if (errored) return next(response) |
There was a problem hiding this comment.
The order of this makes me a little uneasy, but it works.
There was a problem hiding this comment.
Yes trying to keep separate what errors are here.
| throw new Error(`Error in overwrite request: ${err.status} ${err.statusText}`) | ||
| return rerum_res | ||
| }) | ||
| .catch(err => { |
There was a problem hiding this comment.
The rest of them do it, so I thought maybe we did it for a reason. At least doing it to be consistent.
2 participants
No description provided.