Universal: TransferHttpCacheModule does not cache non 20* responses

[babarinde]

🐞 Bug report

What modules are related to this issue?

• [ ] aspnetcore-engine
• [ ] builders
• [x] common
• [ ] express-engine
• [ ] hapi-engine
• [ ] module-map-ngfactory-loader

Is this a regression?

Not sure

Description

First, Thanks for the work you are doing.
I am relatively new with Angular as well as Universal.
It seems that the TransferHttpCacheModule does not cache error responses (i.e. 40, 50) from GET requests and as such the client will make another request to the API since it can't pull it from the cache.

🔬 Minimal Reproduction

🔥 Exception or Error

## 🌍 Your Environment

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 9.0.5
Node: 12.16.1
OS: linux x64

Angular: 9.0.6
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.5
@angular-devkit/build-angular     0.900.5
@angular-devkit/build-optimizer   0.900.5
@angular-devkit/build-webpack     0.900.5
@angular-devkit/core              9.0.5
@angular-devkit/schematics        9.0.5
@angular/cdk                      8.2.3
@angular/cli                      9.0.5
@angular/platform-server          9.0.7
@ngtools/webpack                  9.0.5
@nguniversal/builders             9.1.0
@nguniversal/common               9.0.2
@nguniversal/express-engine       9.0.2
@schematics/angular               9.0.5
@schematics/update                0.900.5
rxjs                              6.5.4
typescript                        3.7.5
webpack                           4.41.2

[CaerusKaru]

@alan-agius4 What do you think about this? Maybe add an option that acts as a filter for request codes?

[bohdanzhylavskyi]

Hi, I also have that problem, don't understand why the error response is not cached.
In that file transfer_http.ts, I see that only successful response will be added to the cache. @CaerusKaru, please, can you explain why the team made a decision to not cache error response?

[CaerusKaru]

I don’t understand your comment. Not everything is a conscious decision. Sometimes v1 is just about getting a solution that works for the majority of cases. My best guess (since this was close to two years ago), is that if you get an HTTP error on the server, you would _want_ to retry on the client to see if you have better luck there. After all, who wants to be stuck in a perpetual error state?

[bohdanzhylavskyi]

@CaerusKaru What about that situation. I have a page with resolver, and error happened in resolver at the server-side. In the resolver error handler, I will redirect the user to the error page. The HTML template with an error page will be sent to the user inside the response. So the user will see the error page template. After all application scripts will be loaded, that same resolver also will be executed in the browser. In that case, maybe a request in the resolver will be executed successfully and the page (not error page) will be shown. Some flickering will happen (change from error template to normal page). Maybe I am doing something wrong.

[CaerusKaru]

Like I said earlier, I think this is worthy of further discussion, that’s why I didn’t close the issue outright. We’ll take a look at all the use cases (and thank you for providing this one!) and see what we can do. We’re not going to just close this and move on, :smile: