[BUG] $onRoutesInit takes a long time since 6.59.6 #1496
Comments
|
And if you upgrade to another version ? |
|
Yes, I pointed v6.59.6 because I wanted to be as accurate as possible on the version number. I tried with v6.60.0, v6.61.1, v6.62.0 and v6.63.2, there is a waiting time in I should mention that with v6.62.0 and v6.63.2, the wait is not as long as with v6.60.0 or v6.61.1, it is 2 seconds (against 4 seconds for v6.60.0 and v6.61.1). |
|
Ok I'll investigate ^^. |
|
Hello @jpc-dnr Can you install the latest version and run the server with the following options please: const platform = await PlatformExpress.bootstrap(Server, {
logger: {
perf: true
}
});
}you should have only the perf log displayed in the terminal. Can send me the result please :) ? See you |
|
Hi Romain, Thank you for that, you can find the log result below. |
|
I also postponed upgrading to @Romakita it seems it somehow relates to Swagger UI. It takes too much time to build |
|
Hoo ok. Thanks for the report. It help me a lot. I would really like to learn how to use profiling tools well. Today it's one of the things that I can't use at all... :( |
|
The latest release should solve this issue. I let this issue open until you haven't confirmed the fix ^^ |
|
@Romakita the issue is still there. Since you use the same method (i.e. private middlewareSwaggerJson(conf: SwaggerSettings) {
return (ctx: PlatformContext) => {
ctx.response.status(200).body(this.swaggerService.getOpenAPISpec(conf));
};
}it blocks the event loop making the service unreachable for ~30s in total in my case. So, notice the new
|
|
Yes I’m arrived on the same conclusion about the deepMerge function. I’ll take a time to find a way to lazyLoad the swagger and optimize the deepMerge function but there nothing magical when the swagger contains a lot of routes xD |
|
Yeah, that's true. Anyway, we should try to avoid building a spec while request executing. Let's start from that, I suggest caching a spec in memory on the |
|
Yes I’ll do that ^^ |
|
Hi! Sorry I was unable to do some testing until today. I can confirm that my original issue is resolved with the new 6.64.1 version. No more delay in the $onRoutesInit hook, thanks! So for me this issue is resolved unless you want to leave it open for other reasons. Thank you again |
|
Unfortunately I have spoken prematurely because now, with 6.64.1, the swagger JSON file is not created as expected. So this seems like another issue or maybe it is related? |
|
The file should be created when the server is ready. |
|
Ok, thanks for the tip. I was doing Swagger UI initialization in the $afterRoutesInit hook, but now I have to do it in the $onReady hook. It is fine for me. Thanks! |
|
@jpc-dnr Yes sorry, this is the only way to resolve the bootstrap perf issue ^^ @derevnjuk Can you confirm that the fix solve also your issue please ;)? |
|
Now I just noticed that the swagger JSON file is not created anymore when bootstrapping for tests ( Is it some unwanted side effect or is there something to add? |
|
Is it necessary to have the generated file during the unit test? |
|
Because unit test should not generate files ^^ |
|
For me I would say "yes" because my tests are verifying that Swagger UI is properly initialized and working, so the JSON files are needed. EDIT: maybe I will see if I can use some hard-coded JSON files for tests |
|
You can emit the $onReady event in this case to generate the swagger json :) |
|
I tried that but it does not generate the swagger json file unfortunately. I also tried something like const mod = await PlatformTest.invoke<SwaggerModule>(SwaggerModule);
mod.$onReady();but it does not work either. |
|
The next release will expose a generateSpecFiles() method on SwaggerModule. You'll be able to do this: There is no reason to not works. The method implementation is the following: generateSpecFiles() {
const promises = this.settings.map(async (conf) => {
const {outFile} = conf;
if (outFile) {
const spec = this.swaggerService.getOpenAPISpec(conf);
return Fs.writeFile(outFile, JSON.stringify(spec, null, 2), {encoding: "utf8"}, () => {});
}
});
return Promise.all(promises);
}So if it doesn't works, I'll need a reproducible example or you will have to debug this method in the SwaggerModule.js in your node_modules :) See you |
|
Thanks for that! It does not work with my project unfortunately (tried with 6.64.2). Tomorrow I will try to reproduce that on a small project to see if this is related to Ts.ED or not. |
|
The problem is not related to Ts.ED, it was related to somehow a race condition between Ts.ED and swagger-stats, because we were reading the swagger.json file in the $onReady hook to give the Swagger spec to swagger-stats. Now we use the directly the SwaggerService service to get the OpenSpec object without relying on the swagger.json file and it works. If you need a reproducible example, you can find it here: https://github.com/jpc-dnr/tsed-1496 Thanks! |
|
@Romakita my issue is still there. See details in #1496 (comment) |
|
But it’s impossible, the spec is lazy loaded during the onReady hook and built only one time for the entire calls (cached). |
|
Hooo.. sorry, I forgot to release the fix ^^' My bad... |
Information
Since the upgrade to v6.59.6, the
$onRoutesInithook is taking a long time (like 4 seconds).We are using in our project the
$onBeforeRoutesInitand$onAfterRoutesInithooks but not the$onRoutesInit.It was working fine with v6.59.5 (
$onRoutesInittakes some milliseconds only to execute).Is there something we are missing? How can we diagnose this issue?
Thanks!
The text was updated successfully, but these errors were encountered: