ui5-community / ui5-ecosystem-showcase

A repository showcasing the UI5 tooling extensibility to combine OSS tools for UI5 application development.
https://ui5-community.github.io/ui5-ecosystem-showcase/
Other
191 stars 92 forks source link

[ui5-middleware-cfdestination] OData v2 requests not triggered, seems cached (version >= 3.1.0) #905

Open qualiture opened 11 months ago

qualiture commented 11 months ago

Describe the bug When using ui5-middleware-cfdestination version 3.3.10, upon initial running of the app with ui5 serve the OData v2 requests are not triggered (we use OData binding in XML view).

To Reproduce Refreshing the app (F5) will load the data, refreshing again will not. I.e. every 'odd' number of refreshes will load the date, every 'even' number of refreshes will not.

_NB 1: In both cases, the <destination>/v2/<my_service>/$metadata request is loaded just fine. However, on every 'even' number of refreshes the HEAD request to <destination>/v2/<my_service> is not triggered, and thus neither are the subsequent OData requests._

NB 2: When disabling the browser cache with browser developer tools open, the OData v2 requests are triggered just fine, hence my suspicion it is cache related.

Expected behavior OData request should be triggered upon initial load, and with every refresh of the app, like it did in version 3.0.1 and below.

petermuessig commented 11 months ago

Hi @qualiture , most probably the cache headers are proxied and not modified. I think this is in general the correct behavior but I could imagine to add an option to disable / remove the cache headers from the response. WDYT?

Can you share the response headers of the cached response?

qualiture commented 11 months ago

Hi @petermuessig, thanks for the quick reply!

And I think you are onto something... initially, I thought the request for $metadata worked fine, because it returned a HTTP 200 response in both instances. However, upon inspection, I noticed the cached response for $metadata, although returning a HTTP 200 response, actually contains the following error response:

Error fetching proxied request: unexpected end of file

I found an old bug mentioning something similar for the http-proxy-middleware repo: https://github.com/chimurai/http-proxy-middleware/issues/658 but apparently it's not fixed yet...

Anyways, these are the response headers (left the uncached, working response, on the right the cached response) for the request for $metadata, shown in DIFF:

image

qualiture commented 4 months ago

Hi @petermuessig,

Now BAS has been updated to use node v20 instead of v18, I've been pressured to see if the more recent ui5-middleware-approuter can be used (we are still using the old ui5-middleware-cfdestination v3.0.1 for local running of our apps)

I did some further investigation, and indeed it seems the dependent http-proxy-middleware does not handle 304 responses correctly (see also https://github.com/chimurai/http-proxy-middleware/issues/381) where the error is thrown when decompressing the response with Zlib

Would it be possible to have an option to disable caching, so the response would be HTTP 200 instead of 304?

petermuessig commented 3 months ago

Hi @qualiture ,

do you now use the ui5-middleware-approuter? I'm asking because for the ui5-middleware-cfdestination the coding has been removed from the repository. It is possible to create a patch but requires to restore the code again - a bit of tedious work... But if you use the new approuter, I may be able to add the option to disable caching - I need to investigate this...

qualiture commented 3 months ago

Hi @petermuessig

yes, am already using the renamed cf-middleware-approuter now 👍🏻

qualiture commented 4 weeks ago

Hi @petermuessig,

As I was in the process of updating NPM dependencies, I felt rebellious, so I removed ui5-middleware-cfdestination and instead of trying ui5-middleware-approuter, I used ui5-middleware-simpleproxy instead. This seems to work perfectly fine, no (proxy) issues encountered.

Is there any reason why we should use ui5-middleware-approuter over ui5-middleware-simpleproxy? NB: We only use this for local development (start CAP service on port 4004, starting UI5 frontend on port 8080) -- when deployed, we use the MTA yaml cross-reference instead

petermuessig commented 3 weeks ago

Hi @qualiture ,

the cfdestination or approuter middleware uses the @sap/approuter to proxy the requests whereas the simpleproxy is using the http-proxy-middleware. The approuter makes sense if you want to use the enhance connectivity features of the approuter (e.g. destinations, authentication, ...). If you don't need this you can also just use the simpleproxy to proxy requests.

Sorry, I missed to follow-up this issue here. I can still look into the option to add a configuration to disable caching for the approuter usage.

qualiture commented 3 weeks ago

Hi @petermuessig, thanks for the clarification. For now, I’ll stuck with the simpleproxy package, so the need to fix this particular issue for ui5-middleware-approuter is less pressing. However, since obviously the authentication part isn’t handled we may run into other issues in the future, so am still hoping this issue could be resolved 😊