Skip to content

Commit 049aa34

Browse files
authored
Timeout generation with an error if the template generation is taking forever (#1539)
- In template-engine.js, produceContent function, timing out the handlebars template handler if it takes more than a certain amount of time for generation because this can lead to ZAP just hanging forever. - Also throwing errors when there is no content generated out of a generation template file - Cleaning up the promises in generateAllTemplates because it was incorrect Fixing the deferred loading and generation in templates with after clause - In the after helper, adding it to the global deferred block array - In template engine adding deferredblock to the context and then later adding it to the content - Removing the entire global.promises.push(syncPromise) which makes the generation of templates hang. Look at the test case which makes this hang - Adding tests to make sure templates no longer hang and also noticing substantial improvement in our generation time after this change - This also allows better usage of promised handlebar helpers where you can now do {{#if promise}} if required - Github: ZAP#792
1 parent d439600 commit 049aa34

File tree

6 files changed

+81
-15
lines changed

6 files changed

+81
-15
lines changed

src-electron/generator/generation-engine.js

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -711,8 +711,6 @@ async function generateAllTemplates(
711711
genTemplateJsonPkg.id
712712
)
713713
let generationTemplates = []
714-
let helperPromises = []
715-
let partialPromises = []
716714
let overridePath = null
717715

718716
let hb = templateEngine.hbInstance()
@@ -746,9 +744,7 @@ async function generateAllTemplates(
746744
// Next load the partials
747745
packages.forEach((singlePkg) => {
748746
if (singlePkg.type == dbEnum.packageType.genPartial) {
749-
partialPromises.push(
750-
templateEngine.loadPartial(hb, singlePkg.category, singlePkg.path)
751-
)
747+
templateEngine.loadPartial(hb, singlePkg.category, singlePkg.path)
752748
}
753749
})
754750

@@ -765,9 +761,7 @@ async function generateAllTemplates(
765761
// Next load the addon helpers which were not yet initialized earlier.
766762
packages.forEach((singlePkg) => {
767763
if (singlePkg.type == dbEnum.packageType.genHelper) {
768-
helperPromises.push(
769-
templateEngine.loadHelper(hb, singlePkg.path, context)
770-
)
764+
templateEngine.loadHelper(hb, singlePkg.path, context)
771765
}
772766
})
773767

@@ -789,10 +783,6 @@ async function generateAllTemplates(
789783
}
790784
})
791785

792-
// And finally go over the actual templates.
793-
await Promise.all(helperPromises)
794-
await Promise.all(partialPromises)
795-
796786
if (options.generateSequentially) {
797787
await util.executePromisesSequentially(generationTemplates, (t) =>
798788
generateSingleTemplate(hb, metaInfo, genResult, t, genTemplateJsonPkg, {
@@ -810,6 +800,11 @@ async function generateAllTemplates(
810800
await Promise.all(templates)
811801
}
812802

803+
if (genResult.hasErrors) {
804+
for (const [key, value] of Object.entries(genResult.errors)) {
805+
console.error(`${key}: ${value}`)
806+
}
807+
}
813808
genResult.partial = false
814809
return genResult
815810
}
@@ -852,6 +847,9 @@ async function generateSingleTemplate(
852847
options
853848
)
854849
for (let result of resultArray) {
850+
if (!result.content) {
851+
console.error(`No content generated for ${result.key}`)
852+
}
855853
genResult.content[result.key] = result.content
856854
genResult.stats[result.key] = result.stats
857855
}

src-electron/generator/helper-zap.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ async function after(options) {
383383
global: this.global,
384384
parent: this
385385
}
386+
this.global.deferredBlocks.push(options.fn)
386387
return options.fn(newContext)
387388
}
388389

src-electron/generator/template-engine.js

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ const precompiledTemplates = {}
7070

7171
const handlebarsInstance = {}
7272

73+
const TEMPLATE_RENDER_TIMEOUT = 90000 // 90 seconds
74+
7375
/**
7476
* Resolves into a precompiled template, either from previous precompile or freshly compiled.
7577
* @param {*} singleTemplatePkg
@@ -185,13 +187,48 @@ async function produceContent(
185187
return null
186188
}
187189
},
188-
stats: {}
190+
stats: {},
191+
deferredBlocks: []
189192
}
190193
}
191194
if (options.initialContext != null) {
192195
Object.assign(context, options.initialContext)
193196
}
194-
let content = await template(context)
197+
let content
198+
// Render the template but if it does not render within
199+
// TEMPLATE_RENDER_TIMEOUT then throw an error instead of just hanging
200+
// forever.
201+
try {
202+
// Attempt to render the template
203+
content = await Promise.race([
204+
template(context),
205+
new Promise((_, reject) =>
206+
setTimeout(
207+
() => reject(new Error('Template rendering timed out')),
208+
TEMPLATE_RENDER_TIMEOUT
209+
)
210+
)
211+
])
212+
// Render deferred blocks
213+
for (let deferredBlock of context.global.deferredBlocks) {
214+
content += await deferredBlock(context)
215+
}
216+
} catch (error) {
217+
// Log the error and throw it
218+
notification.setNotification(
219+
db,
220+
'ERROR',
221+
`Error during template rendering of ${singleTemplatePkg.path}: ` +
222+
error.message,
223+
sessionId,
224+
1
225+
)
226+
console.error(
227+
`Error during template rendering of ${singleTemplatePkg.path}: `,
228+
error
229+
)
230+
throw error
231+
}
195232
return [
196233
{
197234
key:

src-electron/generator/template-util.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,6 @@ async function ensureZclEventSdkExtensions(context, templatePackageId) {
494494
*/
495495
function templatePromise(global, promise) {
496496
let syncPromise = makeSynchronizablePromise(promise)
497-
global.promises.push(syncPromise)
498497
return syncPromise
499498
}
500499
/**

test/gen-matter-1.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,25 @@ test(
274274
"// command: 0x0300 / 0x00 => MoveToHue, test extension: '', isLargeMessage: true"
275275
)
276276
)
277+
278+
// Testing promisedHandlebars when we have {{#if (promise)}} in a template
279+
// Eg: {{#zcl_clusters}}
280+
// {{#zcl_commands_source_server}}
281+
// {{#zcl_command_arguments}}
282+
// {{#if (zcl_command_arguments_count ../index)}}
283+
// zcl command arguments exist for {{../name}} command. It has {{zcl_command_arguments_count ../index}} arguments
284+
// {{else}}
285+
// zcl command arguments do not exist for {{../name}} command.
286+
// {{/if}}
287+
// {{/zcl_command_arguments}}
288+
// {{/zcl_commands_source_server}}
289+
// {{/zcl_clusters}}
290+
expect(simpleTest).toContain(
291+
'zcl command arguments exist for ViewGroupResponse command. It has 5 arguments'
292+
)
293+
expect(simpleTest).toContain(
294+
'zcl command arguments do not exist for AddSceneResponse command.'
295+
)
277296
},
278297
testUtil.timeout.long()
279298
)

test/gen-template/matter/simple-test.zapt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,15 @@ Base type for {{name}} : {{baseType}}
4242
{{/if}}
4343
{{/zcl_commands}}
4444

45+
46+
{{#zcl_clusters}}
47+
{{#zcl_commands_source_server}}
48+
{{#zcl_command_arguments}}
49+
{{#if (zcl_command_arguments_count ../index)}}
50+
zcl command arguments exist for {{../name}} command. It has {{zcl_command_arguments_count ../index}} arguments
51+
{{else}}
52+
zcl command arguments do not exist for {{../name}} command.
53+
{{/if}}
54+
{{/zcl_command_arguments}}
55+
{{/zcl_commands_source_server}}
56+
{{/zcl_clusters}}

0 commit comments

Comments
 (0)