Add response.statusMessage data information in environment logs#853
Add response.statusMessage data information in environment logs#853255kb merged 3 commits intomockoon:mainfrom AndreiOrmanji:feature/852-add-response-status-message-in-environment-logs
Conversation
|
I don't know if I'm allowed to mark any item in checklist as "solved", so I've left it all "as is". |
packages/desktop/src/renderer/app/components/environment-logs/environment-logs.component.html
Outdated
Show resolved
Hide resolved
255kb
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
I run the CI and there are some Prettier formatting issue. Please run Prettier before pushing again. I also add a comment concerning the log label.
|
Now that I am thinking about it, I think it would be better to display the Status message next to the status code, so it's more in line with what other tools like Curl are displaying. What do you think? |
In my optinion,it will "break existing UI consistency" where you have already 'General', 'Headers', 'Body' sections. |
What I meant was to add the message next to the status code to avoid adding a new line, like this: Instead of: I'm not sure if adding the HTTP version is really useful. Wouldn't it be always something like 1.1 as we are using Node.js' http? |
Your varians looks good too :)
It depends on server the requests are sent to. For example, persistent connection (read more at https://developer.mozilla.org/en-US/docs/Web/HTTP/Connection_management_in_HTTP_1.x) is available from 1.1 and higher, so it can be useful to know what version is supported by server. |
|
If you don't mind, maybe display status code and message on one line, I think I prefer 🙂 |
I want to add HTTP version. Can you add an example how it would look like? |
|
Maybe something like: So it's ready for other protocols like web socket when we will implement web sockets 😄. |
|
Sorry for pipeline fails, but I could not set up project on my machine to execute tests from pipeline... |
|
No worries, I used the CI all the time to run the tests 😅 |
|
It seems that node js and express did not provide access to protocol and version to send in response, so I've set values from request, where it was possible. I've refused to use docker. With windows I've the following error on test running: $ npm -v
8.19.2
$ node -v
v19.0.0
$ nvm -v
Running version 1.1.9. |
I would say the same. Maybe remove it and stick to the status message only? It's probably too much hassle for the benefits right now. |
- fixed formatting in packages/desktop/src/renderer/app/components/environment-logs/environment-logs.component.html - changed label for response.statusMessage - updated tests with current fixes Closes #852
|
Thank you for the changes. Everything looks ok, I will merge and add it to the next release (probably later this month). |
|
Thank you very much |



Closes #852
Technical implementation details
I've been added a new property
statusMessageinTransaction.responselocated inpackages\commons\src\models\server.model.tsfor getting the statusMessage in "Logs" tab and some other places:function CreateTransactionin packages\commons-server\src\libs\utils.ts that converts a pair ofRequestandResponseobjects in aTransactionobjectEnvironmentLogResponsein packages\desktop\src\renderer\app\models\environment-logs.model.ts for showing the information from modifiedTransactionin "Logs" tabfunction formatLogin packages\desktop\src\renderer\app\services\data.service.ts for converting theTransactionobject inEnvironmentLogResponseobject.Disclaimer:
I'm not an experienced open software contributor or at least a JS developer, but I want to help you, Mockoon supporters, to make Mockoon better.
Checklist