mui / toolpad

Toolpad: Full stack components and low-code builder for dashboards and internal apps.
https://mui.com/toolpad/
MIT License
964 stars 243 forks source link

Return response on failed requests #610

Closed oliviertassinari closed 2 years ago

oliviertassinari commented 2 years ago

Duplicates

Latest version

Summary 💡

When an API request is not OK

https://github.com/mui/mui-toolpad/blob/479def38e5a01f23aac7da8bb90f33f78e49fe9d/packages/toolpad-app/src/toolpadDataSources/rest/server.ts#L60

It only logs the error:

Screenshot 2022-06-25 at 16 19 56

However, it's not helpful. I would expect something like this (Postman):

Screenshot 2022-06-25 at 16 25 08

https://mui-org.postman.co/workspace/Team-Workspace~a3fd1430-e7a2-482a-8fa2-e4f40a2223c8/request/19543436-85f9898b-a606-4caa-ad96-cb46ba1257ea

or

Screenshot 2022-06-25 at 16 21 24

Examples 🌈

To reproduce

  1. open https://master--toolpad.mui.com/_toolpad/app/cl1c2j0l512939zo6a43575en/editor/pages/cl4tz0p6600083b6ed912vgvf
  2. open the "query"
  3. Run the preview button

For context, it calls https://app.goflightlabs.com/flights

Motivation 🔦

I had to run the project locally to add a console.log to understand what was going wrong. Ideas for follow-ups:

  1. The port 3000 locally conflicts with Material-UI's docs, maybe to use a different one, https://www.notion.so/mui-org/Ports-b113d0d4c76c4db9a0ef99c703d0084c?
  2. Arguably, seeing the request, similar to the Chrome/Firefox Dev Tools would have helped too. I would have realized that I didn't use the UI correctly.
  3. I thought that the "Parameters" were URL query parameters but not at all, they are meant to be used to create dynamic queries. So maybe it would be great to add a new URL query params section. It would be easier to use than building the URL, enforcing that the URL syntax (&, ? =) is correct. Then we could rename "Parameters" to be clearer, and these URL query params could optionally use "Parameters" when we want the query to be reused.
Janpot commented 2 years ago

Just gonna put a more radical idea out there, but what if instead of trying to create a UI that would support every possible use case that people may have, we just give people a code editor and let them write the request together with any possible post-processing themselves?

e.g. you'd just write

export default async () => fetch('/my/api');

and it would run this, serverside, with the baseUrl and headers of the connection automatically supplied. This would give you full freedom to add any pre or post-processing you desire. It'd essentially be a serverless function we allow users to write.

this would also be beneficial implementing the bundle size monitor as that requires chaining two Rest calls. It could be done in a single serverside function.

Janpot commented 2 years ago

Adding a proposal for that here: https://github.com/mui/mui-toolpad/issues/628

prakhargupta1 commented 2 years ago

For an invalid request URL (done through functions data source). I get the following response in Postman:

Image

And this one in Toolpad:

Image

How can we improve response messages?

Janpot commented 2 years ago

There is a network panel now to debug the request that has all this information. Its UX could be improved, tracking that in in https://github.com/mui/mui-toolpad/issues/739.

oliviertassinari commented 2 years ago

I have faced a frustration about this when I worked with Pipedream yesterday. I was calling the Google Calendar API with a wrong end date, but the error message was obscure. It took me a while to figure it out. It was after I made this fix https://github.com/PipedreamHQ/pipedream/pull/4063. I almost gave up. So 👍 to stay attentive to recovery experience on mistakes.

I will double check, but I suspect that based on Jan comment we can close this issue, you will never get completely stuck with the new network panel 😍

oliviertassinari commented 2 years ago

From my end, we can close, the pain I raised is solved thanks to:

Screenshot 2022-08-25 at 19 07 03

The only feedback I would leave is the timeline view is a distraction. I think that listing the previous API calls is enough, it would be better to hide the timeline.

prakhargupta1 commented 2 years ago

Ok, I'll close this issue as the Network tab gives all of the needed info.