Avoid infinite loop and fatal errors for invalid table data #296
Conversation
speedytwenty
changed the title
speedytwenty
changed the title
| const { x, y } = rowSpanCell; | ||
| warn(`${x}-${y}: Expected empty array for row ${line - 1} (line ${line}).`); | ||
| } | ||
| insertCell(rowSpanCell, table[rowSpanCell.y] || []); |
There was a problem hiding this comment.
Phantom row will never be output. But a fatal error is avoided.
| @@ -146,7 +151,7 @@ const { ColSpanCell, RowSpanCell } = Cell; | |||
| cell.x = opts.x; | |||
| cell.y = opts.y; | |||
| warn(`Missing cell at ${cell.y}-${cell.x}.`); | |||
| insertCell(cell, table[y]); | |||
| insertCell(cell, table[y] || []); | |||
There was a problem hiding this comment.
Phantom row will never be output. But a fatal error is avoided.
| @@ -110,7 +115,7 @@ const { ColSpanCell, RowSpanCell } = Cell; | |||
| let colSpanCell = new ColSpanCell(); | |||
| colSpanCell.x = cell.x + k; | |||
| colSpanCell.y = cell.y; | |||
| cellColumns.splice(columnIndex + 1, 0, colSpanCell); | |||
| cellColumns.splice(colSpanCell.x, 0, colSpanCell); | |||
There was a problem hiding this comment.
This fixes the (known) infinite loop that can be caused by invalid-table-data.
| [], | ||
| [{ content: 'C', colSpan: 3 }] | ||
| ); | ||
| expect(() => table.toString()).not.toThrow(); |
There was a problem hiding this comment.
This expectation induces an infinite loop before the "fix" in this pr.
| ); | ||
| expect(() => table.toString()).not.toThrow(); | ||
| // This expectation can be dropped if the expectation in the code changes | ||
| expect(table.messages).toContain('2-2: Expected empty array for row 1 (line 2).'); |
There was a problem hiding this comment.
Silent solution to the missing row. Future solutions can add the missing row to correct the output.
|
Preferably, this PR should address the loop that caused the infinite loop. As is, this PR only avoids the infinite loop with known inductions, but, there may remain other circumstances for which it could occur that could be further avoided by fatal errors or by making the loop assuredly finite. I suppose it would be my presumption that cli-table3 intends to be forgiving and that the latter recourse of making the loop finite—instead of falling into an infinite loop or detecting it and throwing an error—is preferable with the original intentions of cli-table3. It's easy enough to make the questionable loop finite so I'll strive to incorporate that—but I'm not sure how to test the difference. |
It's possible to induce errors—including infinite loops—with invalid table data where it seems the intention of cli-table3 is to be forgiving and "fill-in-the-blanks".
Was hitting an infinite loop in
Cell.drawEmpty()when generating tables with #294 which has/is leading to uncovering further problematic error data that can be recovered from and debugged via #288. Both simple and complicated rowSpan and colSpan mixtures can produce fatal errors or infinite loops.1. cli-table3 should avoid falling into an infinite loop with invalid table data.
The first commit includes "failing"—non-completing tests due to the infinite loop.
The infinite loop occurs after ColSpanCells are filled in for invalid-table data in the incorrect order. The second commit shows a one-liner hack-fix that corrects the insertion order of ColSpanCells—with failing tests remaining for 2 (see below). Yet, I think it's likely worth expanding on this to detect the infinite loop and throw a fatal error should there be other cases that this may occur. Alternatively, the obscure loop in question can be easily altered to avoid infinitum altogether.
2. cli-table3 should avoid fatal errors for invalid table data
The third commit is a fix that passes the tests for known fatal errors that can be caused by invalid data.
While there are more elaborate resolutions for both cases that include more appropriately filling in invalid-table data, the current PR does not [knowingly] change the resulting output of invalid table data that otherwise renders a faulty table and is therefore not a breaking change.
Refinements to filling in invalid data more deliberately can be made, but would likely induce a breaking change (for invalid table data).