Skip to content
This repository was archived by the owner on Dec 17, 2024. It is now read-only.

Commit c88fd78

Browse files
starpitstarpit
committed
multiple fixes for error handling with drilldown from activation views
Fixes #799
1 parent f81701d commit c88fd78

File tree

7 files changed

+103
-62
lines changed

7 files changed

+103
-62
lines changed

‎app/content/js/bottom-stripe.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ exports.addModeButtons = (modes, entity, options) => {
183183
const bottomStripe = document.querySelector(css.modeContainer)
184184

185185
// for going back
186-
bottomStripe.addModeButtons = (modes, entity, show) => {
186+
const addModeButtons = (modes, entity, show) => {
187187
const bottomStripe = document.querySelector(css.modeContainer)
188188
ui.removeAllDomChildren(bottomStripe)
189189

@@ -202,13 +202,13 @@ exports.addModeButtons = (modes, entity, options) => {
202202
// to avoid stale buttons from showing up while the new view renders
203203
ui.removeAllDomChildren(bottomStripe)
204204

205-
return () => bottomStripe.addModeButtons(modes, entity, show)
205+
return () => addModeButtons(modes, entity, show)
206206
}
207207
}
208208

209209
const defaultMode = modes && modes.find(({defaultMode}) => defaultMode),
210210
show = options && options.show || (defaultMode && (defaultMode.mode || defaultMode.label))
211-
bottomStripe.addModeButtons(modes, entity, show)
211+
addModeButtons(modes, entity, show)
212212

213213
if (!options || !options.preserveBackButton) {
214214
const backContainer = document.querySelector(css.backContainer)

‎app/content/js/picture-in-picture.js‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
* limitations under the License.
1515
*/
1616

17-
const util = require('util'),
17+
const debug = require('debug')('pip'),
1818
bottomStripe = require('./bottom-stripe')
1919

2020
const _highlight = op => highlightThis => {
2121
if (highlightThis) {
22-
if (util.isArray(highlightThis)) {
22+
if (Array.isArray(highlightThis)) {
2323
highlightThis.forEach(_ => _.classList[op]('picture-in-picture-highlight'))
2424
} else {
2525
highlightThis.classList[op]('picture-in-picture-highlight')
@@ -34,6 +34,8 @@ const highlight = _highlight('add')
3434
*
3535
*/
3636
const restore = (pippedContainer, sidecarClass, capturedHeaders, highlightThis, escapeHandler, options) => () => {
37+
debug('restore')
38+
3739
const sidecar = document.getElementById('sidecar'),
3840
parent = options && options.parent || sidecar.querySelector('.custom-content')
3941

@@ -107,6 +109,8 @@ const pip = (container, capturedHeaders, highlightThis, returnTo, options) => {
107109
backContainer.classList.remove('has-back-button')
108110
}
109111
}
112+
113+
return restoreFn
110114
}
111115

112116
/**
@@ -178,25 +182,16 @@ module.exports = (command, highlightThis, container, returnTo, options) => event
178182
const modeButtons = document.querySelector(bottomStripe.css.modeContainer).capture
179183
const capturedFooter = capture(bottomStripe.css.buttons, modeButtons && modeButtons())
180184

181-
if (container && !alreadyPipped) {
182-
// make the transition
183-
pip(container, [capturedHeader, capturedHeader2, capturedHeader3, capturedHeader4, capturedFooter], highlightThis, returnTo, options)
184-
185-
/*} else if (alreadyPipped) {
186-
// for real pip... if the transition has already been made
187-
const currentHighlightThis = alreadyPipped.querySelectorAll('.picture-in-picture-highlight')
188-
if (currentHighlightThis) {
189-
for (let idx = 0; idx < currentHighlightThis.length; idx++) {
190-
dehighlight(currentHighlightThis[idx])
191-
}
192-
}*/
193-
}
185+
// make the transition
186+
const restoreFn = container && !alreadyPipped
187+
? pip(container, [capturedHeader, capturedHeader2, capturedHeader3, capturedHeader4, capturedFooter], highlightThis, returnTo, options)
188+
: () => true
194189

195190
highlight(highlightThis)
196191

197192
// now we can safely begin executing the command
198-
return typeof command === 'string'
199-
? repl.pexec(command, { preserveBackButton: true })
200-
: typeof command === 'function' ? command()
193+
debug('executing command', command)
194+
return typeof command === 'string' ? repl.pexec(command, { preserveBackButton: true, rethrowErrors: true, reportErrors: true }).catch(restoreFn)
195+
: typeof command === 'function' ? command().catch(restoreFn)
201196
: ui.showEntity(command, { preserveBackButton: true })
202197
}

‎app/content/js/repl.js‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -920,19 +920,32 @@ self.exec = (commandUntrimmed, execOptions) => {
920920
}
921921
})
922922
.catch(err => {
923-
console.error(err)
923+
console.error('error in command execution', err)
924924

925925
if (ui.headless) {
926926
throw err
927927
} else {
928928
// indicate that the command was NOT successfuly completed
929929
evaluator.error(err)
930930

931-
if (execOptions && execOptions.failWithUsage) {
931+
// how should we handle the error?
932+
const returnIt = execOptions && execOptions.failWithUsage, // return to caller; it'll take care of things from now
933+
rethrowIt = execOptions && execOptions.rethrowErrors, // rethrow the exception
934+
reportIt = execOptions && execOptions.reportErrors // report it to the user via the repl
935+
936+
if (returnIt) {
937+
debug('returning command execution error')
932938
return err
933-
} else if (!nested) {
939+
} else if (!nested && !rethrowIt) {
940+
debug('reporting command execution error to user via repl')
934941
ui.oops(block, nextBlock)(err)
935942
} else {
943+
debug('rethrowing command execution error')
944+
if (reportIt) {
945+
// maybe the caller also wants us to report it via the repl?
946+
debug('also reporting command execution error to user via repl')
947+
ui.oops(block, nextBlock)(err)
948+
}
936949
throw err
937950
}
938951
}
@@ -945,7 +958,7 @@ self.exec = (commandUntrimmed, execOptions) => {
945958
throw e
946959
}
947960

948-
console.error(e)
961+
console.error('catastrophic error in repl', e)
949962

950963
const blockForError = block || ui.getCurrentProcessingBlock()
951964

‎app/plugins/modules/activation-visualizations/lib/drilldown.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
* limitations under the License.
1515
*/
1616

17-
exports.drilldownWith = (returnTo, command, highlightThis) => event => {
17+
exports.drilldownWith = (returnTo, command, highlightThis, callThese=[]) => event => {
18+
// invoke any precursor functions
19+
callThese.forEach(_ => _())
20+
1821
const container = document.querySelector('#sidecar .custom-content .activation-viz-plugin')
1922
return ui.pictureInPicture(command, highlightThis, container, returnTo)(event)
2023
}

‎app/plugins/modules/activation-visualizations/lib/grid.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ const _drawGrid = (options, {sidecar, leftHeader, rightHeader}, content, groupDa
265265
labelInner.appendChild(labelAction)
266266
labelAction.innerText = actionName
267267
labelAction.className = 'clickable grid-label-part'
268-
labelAction.onclick = drilldownWith(viewName, `grid ${optionsToString(options)} --zoom 1 --name "${group.path}"`)
268+
labelAction.onclick = drilldownWith(viewName, `grid "${group.path}" ${optionsToString(options)}`)
269269
}
270270

271271
// render the grid

‎app/plugins/modules/activation-visualizations/lib/table.js‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,15 @@ const tableModes = choices.map((choice, idx) => {
4949
}
5050
}
5151
}).concat([
52+
// this is the bottom stripe button that toggles whether outliers are shown
53+
// note how we specify that this is a radio button, i.e. a toggler
5254
{ mode: 'outliers',
5355
fontawesome: 'fas fa-exclamation',
5456
flush: 'right',
5557
balloon: 'Include outlier activations with very high latency',
56-
actAsButton: true, selected: false,
57-
radioButton: true, // this is a separate radio button
58+
actAsButton: true,
59+
radioButton: true,
60+
selected: false,
5861
direct: state => {
5962
const showOutliers = !state.showOutliers
6063
state.showOutliers = showOutliers
@@ -164,6 +167,10 @@ const _drawTable = (options, header, content, groupData, eventBus, sorter=statDa
164167
xAxisFocusLabelRange.style.width = 0
165168

166169
/** Render a selected range on the x axis */
170+
const xAxisResetFocus = barWrapper => () => {
171+
content.classList.remove('x-axis-focus')
172+
barWrapper.classList.remove('focus')
173+
}
167174
const xAxisToggleFocus = ({barWrapper, this25, this75, left, right}) => {
168175
const inFocus = content.classList.toggle('x-axis-focus')
169176
barWrapper.classList.toggle('focus')
@@ -346,11 +353,6 @@ const _drawTable = (options, header, content, groupData, eventBus, sorter=statDa
346353
const indicators = barWrapper.querySelectorAll('.stat-indicator')
347354
indicators.forEach(indicator => barWrapper.removeChild(indicator))
348355
}
349-
350-
// drill down to grid view; note how we pass through a name filter
351-
// query, to filter based on the clicked-upon row
352-
cell.onclick = drilldownWith(viewName, `grid "${group.path}" ${optionsToString(options)} ${splitOptions}`)
353-
354356
const this25 = group.statData.n[stat25],
355357
thisMedian = group.statData.n['50'],
356358
this75 = group.statData.n[stat75],
@@ -367,12 +369,17 @@ const _drawTable = (options, header, content, groupData, eventBus, sorter=statDa
367369
bar.style.width = percent(right - left)
368370

369371
// fancy focus, to show the extent of the bar on the x axis!
370-
const doFocus = () => xAxisToggleFocus({barWrapper, this25, this75, left, right}),
372+
const resetFocus = xAxisResetFocus(barWrapper),
373+
doFocus = () => xAxisToggleFocus({barWrapper, this25, this75, left, right}),
371374
focus = dom => {
372375
dom.onmouseenter = doFocus
373376
dom.onmouseleave = doFocus
374377
}
375378

379+
// drill down to grid view; note how we pass through a name filter
380+
// query, to filter based on the clicked-upon row
381+
cell.onclick = drilldownWith(viewName, `grid "${group.path}" ${optionsToString(options)} ${splitOptions}`, undefined, [resetFocus])
382+
376383
// install the fancy focus handlers
377384
focus(bar)
378385

‎tests/tests/passes/05/activation-table-view.js‎

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,33 +46,34 @@ const _openTableExpectCountOf = function(ctx, expectedCount, expectedErrorRate,
4646
outlierDots = `${view} .outlier-dot`
4747

4848
const once = (iter, resolve, reject) => cli.do(cmd, ctx.app)
49-
.then(cli.expectOK)
50-
.then(sidecar.expectOpen)
51-
.then(() => ctx.app.client.scroll(row))
52-
.then(() => ctx.app.client.getText(successCell))
53-
.then(actualCount => assert.equal(actualCount, expectedCount))
54-
55-
.then(() => ctx.app.client.getAttribute(failureCell, 'data-failures'))
56-
.then(actualErrorRate => assert.equal(actualErrorRate, expectedErrorRate))
57-
58-
// hover over a bar, expect focus labels
59-
.then(() => ctx.app.client.moveToObject(medianDot, 1, 1))
60-
.then(() => ctx.app.client.waitForVisible(focusLabel))
61-
62-
// click outliers button
63-
.then(() => {
64-
if (expectedCount > 8) {
65-
return ctx.app.client.click(outliersButton)
66-
.then(() => ctx.app.client.waitForVisible(outlierDots))
67-
}
49+
.then(cli.expectOKWithCustom({ passthrough: true}))
50+
.then(N => {
51+
// we'll return this N at the end; this is the data-input-count of the prompt that executed our cmd
52+
return ctx.app.client.scroll(row)
53+
.then(() => ctx.app.client.getText(successCell))
54+
.then(actualCount => assert.equal(actualCount, expectedCount))
55+
56+
.then(() => ctx.app.client.getAttribute(failureCell, 'data-failures'))
57+
.then(actualErrorRate => assert.equal(actualErrorRate, expectedErrorRate))
58+
59+
// hover over a bar, expect focus labels
60+
.then(() => ctx.app.client.moveToObject(medianDot, 1, 1))
61+
.then(() => ctx.app.client.waitForVisible(focusLabel))
62+
63+
// click outliers button
64+
.then(() => {
65+
if (expectedCount > 8) {
66+
return ctx.app.client.click(outliersButton)
67+
.then(() => ctx.app.client.waitForVisible(outlierDots))
68+
}
69+
})
70+
71+
/*.then(() => ctx.app.client.getAttribute(`${ui.selectors.SIDECAR_CUSTOM_CONTENT} tr[data-action-name="${actionName}"] .cell-stat`, 'data-value'))
72+
.then(stats => assert.equal(stats.length, 5) && stats.reduce((okSoFar,stat) => ok && isInteger(stat), true))*/
73+
74+
// return the repl prompt count
75+
.then(() => resolve(N))
6876
})
69-
70-
/*.then(() => ctx.app.client.getAttribute(`${ui.selectors.SIDECAR_CUSTOM_CONTENT} tr[data-action-name="${actionName}"] .cell-stat`, 'data-value'))
71-
.then(stats => assert.equal(stats.length, 5) && stats.reduce((okSoFar,stat) => ok && isInteger(stat), true))*/
72-
73-
// return the selector
74-
.then(() => `${ui.selectors.SIDECAR_CUSTOM_CONTENT} tr[data-action-name="${actionName}"]`)
75-
.then(resolve)
7677
.catch(err => {
7778
if (iter < 10) {
7879
if (err.type !== 'NoSuchElement') {
@@ -160,7 +161,7 @@ describe('Activation table visualization', function() {
160161
openTableExpectCountOf(this, 3, 1, 'summary --batches 10')
161162

162163
it('should open table, click on a failure cell, and show grid', () => _openTableExpectCountOf(this, 3, 1, 'summary --batches 10')
163-
.then(() => `${ui.selectors.SIDECAR_CUSTOM_CONTENT} tr[data-action-name="${actionName}"] .cell-errors`)
164+
.then(() => `${ui.selectors.SIDECAR_CUSTOM_CONTENT} tr[data-action-name="${actionName}"] .cell-errors`) // the failure cell for the action's row
164165
.then(selector => this.app.client.scroll(selector)
165166
.then(this.app.client.click(selector)))
166167
.then(() => this.app)
@@ -190,4 +191,26 @@ describe('Activation table visualization', function() {
190191
.catch(common.oops(this)))
191192

192193
openTableExpectCountOf(this, 46, 4, `summary ${actionName}`) // 46 successful activations, 4 of which failed
194+
195+
// finally, test error handling: delete action, open summary,
196+
// click on the action name, except the summary to still be there
197+
it(`should delete ${actionName}`, () => cli.do(`wsk action delete ${actionName}`, this.app)
198+
.then(cli.expectOK)
199+
.catch(common.oops(this)))
200+
201+
it('should open table, click on the deleted action, and still show table view', () => _openTableExpectCountOf(this, 46, 4, `summary ${actionName}`)
202+
.then(N => {
203+
// N is the data-input-count of the block that executed the command
204+
const selector = `${ui.selectors.SIDECAR_CUSTOM_CONTENT} tr[data-action-name="${actionName}"] .cell-label .clickable` // the name cell for the deleted action
205+
return this.app.client.scroll(selector)
206+
.then(this.app.client.click(selector)) // click on it
207+
.then(count => ({ app: this.app, count: N + 1 })) // the command+1 had better have a 404
208+
.then(cli.expectError(404)) // the repl should report the action not found error
209+
})
210+
.then(() => this.app.client.waitUntil(() => {
211+
return sidecar.expectOpen(this.app)
212+
.then(sidecar.expectMode('summary')) // and the summary had better still be open
213+
}))
214+
.catch(common.oops(this)))
215+
193216
})

0 commit comments

Comments
 (0)