- Can you reproduce the bug at https://try.gitea.io:
- [x] Yes (URL)
- [ ] No
- [ ] Not relevant
Description
We have damaged graph on some projects. This is very disturbing. Thanks.
Screenshots
kinbug
a-n-dokuchaev
All 16 comments
Could you give more details?
lunny
on 20 Jun 2020
👍1
The graph is disappearing after doing some merges or pushing local branches. It's the only thing we know. We found 5/100+ of our repos with the same state as on the try.gitea.io.
a-n-dokuchaev
on 20 Jun 2020
👍1
Looking at the browser inspector console I see:
jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1 jQuery.Deferred exception: Cannot read property 'color' of undefined TypeError: Cannot read property 'color' of undefined
at https://try.gitea.io/js/gitgraph.js:1:4466
at n (https://try.gitea.io/js/gitgraph.js:1:4703)
at https://try.gitea.io/js/index.js?v=798fb62373ca0ec978de1f0f5e229f0f:1:67748
at l (https://try.gitea.io/js/index.js?v=798fb62373ca0ec978de1f0f5e229f0f:1:255165)
at Generator._invoke (https://try.gitea.io/js/index.js?v=798fb62373ca0ec978de1f0f5e229f0f:1:254918)
at Generator.forEach.e.<computed> [as next] (https://try.gitea.io/js/index.js?v=798fb62373ca0ec978de1f0f5e229f0f:1:255522)
at n (https://try.gitea.io/js/index.js?v=798fb62373ca0ec978de1f0f5e229f0f:1:553357)
at s (https://try.gitea.io/js/index.js?v=798fb62373ca0ec978de1f0f5e229f0f:1:553568) undefined
C.Deferred.exceptionHook @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
c @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
(anonymous) @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
Promise.then (async)
l @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
c @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
setTimeout (async)
(anonymous) @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
u @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
fireWith @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
fire @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
u @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
fireWith @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
ready @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
_ @ jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1
jquery.js?v=798fb62373ca0ec978de1f0f5e229f0f:1 Uncaught TypeError: Cannot read property 'color' of undefined
at gitgraph.js:1
at n (gitgraph.js:1)
at gitgraph.js:12
at l (runtime.js:45)
at Generator._invoke (runtime.js:274)
at Generator.forEach.e.<computed> [as next] (runtime.js:97)
at n (asyncToGenerator.js:3)
at s (asyncToGenerator.js:25)
zeripath
on 20 Jun 2020
running with: make watch-frontend in the console I see:
jquery.js?v=d6ca3c6360663dfd3a697d1a6e4ed792:4150 Uncaught TypeError: Cannot read property 'color' of undefined
at draw (gitgraph.js:519)
at gitGraph (gitgraph.js:557)
at _callee$ (to-absolute-index.js:11)
at tryCatch (to-absolute-index.js:11)
at Generator.invoke [as _invoke] (to-absolute-index.js:11)
at Generator.prototype.<computed> [as next] (to-absolute-index.js:11)
at asyncGeneratorStep (to-absolute-index.js:11)
at _next (to-absolute-index.js:11)
Line 519 (to the browser) appears to match up with:
zeripath
on 20 Jun 2020
It appears the problem is that the git graph is not in the format that this js function seems to expect.
The js seems to expect the use of underscores and not hyphens...
If I bodge in code to handle hyphens the problem goes away.
Now I'm not sure why this is different but my build of git 1.7.2 appears to use hyphens so perhaps this has always been broken?!
Edit:
I suspect it has
zeripath
on 20 Jun 2020
Another damaged graph: https://try.gitea.io/ahaha/gitgraph/graph
ahttvy
on 22 Jun 2020
Hello! 2/5 of our repos still damaged. @ahttvy may be right
Another damaged graph: https://try.gitea.io/ahaha/gitgraph/graph
a-n-dokuchaev
on 23 Jun 2020
sigh...
zeripath
on 23 Jun 2020
OK so that's a different but related issue - here's another bugfix... I suspect this code needs to be completely rewritten.
zeripath
on 24 Jun 2020
The last one - https://try.gitea.io/ahaha/qtconnectivity/graph
a-n-dokuchaev
on 26 Jun 2020
Ugh this one is a bit more difficult. I'll have to take a deeper look to understand what the code is doing.
zeripath
on 27 Jun 2020
The below patch will prevent the graph from erroring out but doesn't fix the underlying problem:
diff --git a/web_src/js/vendor/gitgraph.js b/web_src/js/vendor/gitgraph.js
index 0cf5d0f75..7a5777598 100644
--- a/web_src/js/vendor/gitgraph.js
+++ b/web_src/js/vendor/gitgraph.js
@@ -376,7 +376,18 @@ export default function gitGraph(canvas, rawGraphList, config) {
flows.splice(colomnIndex, 0, genNewFlow());
}
- color = flows[colomnIndex].color;
+ if (colomnIndex < flows.length) {
+ color = flows[colomnIndex].color;
+ } else {
+ console.error({
+ message: "Missing flow!",
+ colomnIndex,
+ colomn,
+ prevColomn,
+ flowSwapPos,
+ });
+ color = "#000";
+ }
switch (colomn) {
case '-':
OK I think this 3rd problem is sufficiently difficult to solve that it might be that I just have to rewrite this code.
zeripath
on 1 Jul 2020
So I think I will have to rewrite the gitgraph.js to look something like this to fix this final problem:
Broken JS
/* This is a customized version of https://github.com/bluef/gitgraph.js/blob/master/gitgraph.js
Changes include conversion to ES6 and linting fixes */
/*
* @license magnet:?xt=urn:btih:c80d50af7d3db9be66a4d0a86db0286e4fd33292&dn=bsd-3-clause.txt BSD 3-Clause
* Copyright (c) 2011, Terrence Lee <[email protected]>
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* * Neither the name of the <organization> nor the
* names of its contributors may be used to endorse or promote products
* derived from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
* DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
* ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
export default function gitGraph(canvas, rawGraphList, config) {
if (!canvas.getContext) {
return;
}
if (typeof config === 'undefined') {
config = {
unitSize: 20,
lineWidth: 3,
nodeRadius: 4
};
}
const nextFlows = [];
const graphList = [];
const ctx = canvas.getContext('2d');
const devicePixelRatio = window.devicePixelRatio || 1;
const backingStoreRatio = ctx.webkitBackingStorePixelRatio
|| ctx.mozBackingStorePixelRatio
|| ctx.msBackingStorePixelRatio
|| ctx.oBackingStorePixelRatio
|| ctx.backingStorePixelRatio || 1;
const ratio = devicePixelRatio / backingStoreRatio;
const init = function () {
let maxWidth = 0;
let i;
const l = rawGraphList.length;
let row;
let midStr;
for (i = 0; i < l; i++) {
midStr = rawGraphList[i].replace(/\s+/g, ' ').replace(/^\s+|\s+$/g, '');
midStr = midStr.replace(/(--)|(-\.)/g,'-')
maxWidth = Math.max(midStr.replace(/(_|\s)/g, '').length, maxWidth);
row = midStr.split('');
graphList.unshift(row);
}
const width = maxWidth * config.unitSize;
const height = graphList.length * config.unitSize;
canvas.width = width * ratio;
canvas.height = height * ratio;
canvas.style.width = `${width}px`;
canvas.style.height = `${height}px`;
ctx.lineWidth = config.lineWidth;
ctx.lineJoin = 'round';
ctx.lineCap = 'round';
ctx.scale(ratio, ratio);
};
const genRandomStr = function () {
const chars = '0123456789ABCDEF';
const stringLength = 6;
let randomString = '', rnum, i;
for (i = 0; i < stringLength; i++) {
rnum = Math.floor(Math.random() * chars.length);
randomString += chars.substring(rnum, rnum + 1);
}
return randomString;
};
const findFlow = function (id) {
let i = nextFlows.length;
while (i-- && nextFlows[i].id !== id);
return i;
};
const findColomn = function (symbol, row) {
let i = row.length;
while (i-- && row[i] !== symbol);
return i;
};
const findBranchOut = function (row) {
if (!row) {
return -1;
}
let i = row.length;
while (i--
&& !(row[i - 1] && row[i] === '/' && row[i - 1] === '|')
&& !(row[i - 2] && row[i] === '_' && row[i - 2] === '|'));
return i;
};
const findLineBreak = function (row) {
if (!row) {
return -1;
}
let i = row.length;
while (i--
&& !(row[i - 1] && row[i - 2] && row[i] === ' ' && row[i - 1] === '|' && row[i - 2] === '_'));
return i;
};
const genNewFlow = function () {
let newId;
do {
newId = genRandomStr();
} while (findFlow(newId) !== -1);
return { id: newId, color: `#${newId}` };
};
// Draw methods
const drawLine = function (moveX, moveY, lineX, lineY, color) {
ctx.strokeStyle = color;
ctx.beginPath();
ctx.moveTo(moveX, moveY);
ctx.lineTo(lineX, lineY);
ctx.stroke();
};
const drawLineRight = function (x, y, color) {
drawLine(x, y + config.unitSize / 2, x + config.unitSize, y + config.unitSize / 2, color);
};
const drawLineUp = function (x, y, color) {
drawLine(x, y + config.unitSize / 2, x, y - config.unitSize / 2, color);
};
const drawNode = function (x, y, color) {
ctx.strokeStyle = color;
drawLineUp(x, y, color);
ctx.beginPath();
ctx.arc(x, y, config.nodeRadius, 0, Math.PI * 2, true);
ctx.fill();
};
const drawLineIn = function (x, y, color) {
drawLine(x + config.unitSize, y + config.unitSize / 2, x, y - config.unitSize / 2, color);
};
const drawLineOut = function (x, y, color) {
drawLine(x, y + config.unitSize / 2, x + config.unitSize, y - config.unitSize / 2, color);
};
const draw = function (graphList) {
let x, y, i;
y = (canvas.height / ratio) - config.unitSize * 0.5;
let flows = new Array(graphList[0].length);
// Generate nextFlows for the first row - this is guaranteed to be: "(\| )*(\*)( \|)*"
for (i = 0; i < graphList[0].length; i++) {
if (graphList[0][i] === ' ') {
continue;
}
flows[i] = genNewFlow();
}
// Draw the first row
for (let idx = 0; idx < graphList[0].length; idx++) {
x = config.unitSize * 0.5 * (idx + 1);
const column = graphList[0][idx];
switch (column) {
case '*':
drawNode(x, y, flows[idx].color);
break;
case ' ':
break;
case '|':
drawLineUp(x, y, flows[idx].color);
break;
default:
console.error(`Unexpected symbol in first row[${idx}] = '${column}'`);
}
}
for (i = 1; i < graphList.length; i++) {
// Done previous row - step up the row
y -= config.unitSize;
const currentRow = graphList[i];
const prevRow = graphList[i-1];
let nextFlows = new Array(currentRow.length);
for (let idx = 0; idx < currentRow.length; idx++) {
switch (currentRow[idx]) {
case '|':
case '*':
if (idx+1 < currentRow.length &&
(currentRow[idx+1] === '/' || currentRow[idx+1] === '_') &&
idx < prevRow.length &&
(prevRow[idx] === '|' || prevRow[idx] === '*') &&
prevRow[idx-1] === '/' ) {
// If ' |/' keep the '|' flow
// '/|'
nextFlows[idx] = flows[idx];
} else if (idx+1 < currentRow.length &&
(currentRow[idx+1] === '/' || currentRow[idx+1] === '_') &&
idx -1 < prevRow.length &&
prevRow[idx-1] === '/') {
// If ' |/' take the '/' flow
// '/ ?'
nextFlows[idx] = flows[idx-1];
flows[idx-1] = genNewFlow();
} else if (idx - 1 < prevRow.length &&
prevRow[idx-1] === '/') {
// if ' |' take the '/' flow
// '/ '
nextFlows[idx] = flows[idx-1];
flows[idx-1] = genNewFlow();
} else if (idx < prevRow.length &&
(prevRow[idx] === '|' || prevRow[idx] === '*')) {
// If '|' OR '|' take the '|' flow
// '|' '*'
nextFlows[idx] = flows[idx];
flows[idx] = genNewFlow();
} else if (idx +1 < prevRow.length &&
prevRow[idx+1] === '\\') {
// If '| ' keep the '\' flow
// ' \'
nextFlows[idx] = flows[idx + 1];
} else {
// else just create a new flow - probably this is an error...
nextFlows[idx] = genNewFlow();
}
break;
case '/':
if (idx > 0 &&
currentRow[idx-1] === '_') {
// if '_/' keep the '_' flow
nextFlows[idx] = nextFlows[idx-1];
} else if (idx > 1 &&
(currentRow[idx-1] === '|' || currentRow[idx-1] === '*') &&
currentRow[idx-2] === '_') {
// If '_|/' keep the '_' flow
nextFlows[idx] = nextFlows[idx-2];
} else if (i > 1 &&
currentRow[idx-1] === '|' &&
idx-2 < prevRow.length &&
prevRow[idx-2] === '/') {
// If '?|/' keep the '/' flow - ('|' will have caused regeneration as necessary)
// '/??'
nextFlows[idx] = flows[idx-2];
} else if (idx > 0 &&
currentRow[idx-1] === ' ' &&
idx-1 < prevRow.length &&
prevRow[idx-1] === '/') {
// If ' /' - keep the '/' flow, but transform to '|' due to our spacing
// '/ '
nextFlows[idx] = flows[idx-1];
currentRow[idx] = '|';
} else if (idx > 0 &&
currentRow[idx-1] === ' ' &&
idx-1 < prevRow.length &&
prevRow[idx-1] === '|') {
// If ' /' - keep the '|' flow
// '| '
nextFlows[idx] = flows[idx-1];
} else if (idx < prevRow.length &&
prevRow[idx] === '\\') {
// If '/' <- Not sure this ever happens... but keep the '\' flow
// '\'
nextFlows[idx] = flows[idx];
} else {
// Otherwise just generate a new flow and wait for bug-reports...
nextFlows[idx] = genNewFlow();
}
break;
case '\\':
if (prevRow[idx] === '/') {
// If '\?' take the '/' flow
// '/?'
nextFlows[idx] = flows[idx];
flows[idx] = genNewFlow();
} else if (idx+1 < prevRow.length &&
prevRow[idx+1] === '\\') {
// If '\?' take the '\' flow and reassign to |
// ' \'
nextFlows[idx] = flows[idx+1];
currentRow[idx] = '|';
flows[idx+1] = genNewFlow(); // <- prevents us having to think too hard about '\|/'
} else if (idx+1 < prevRow.length &&
(prevRow[idx+1] === '|' || prevRow[idx+1] === '*')) {
// If '\?' take the '|' flow
// ' |'
nextFlows[idx] = flows[idx+1];
flows[idx+1] = genNewFlow(); // <- prevents us having to think too hard about '\|/'
} else {
// Otherwise just generate a new flow and wait for bug-reports...
nextFlows[idx] = genNewFlow();
}
break;
case '_':
if (idx > 0 &&
currentRow[idx-1] === '_') {
// if '__' keep the '_' flow
nextFlows[idx] = nextFlows[idx-1];
} else if (idx > 1 &&
currentRow[idx-1] === '|' &&
currentRow[idx-2] === '_') {
// if '_|_' keep the '_' flow
nextFlows[idx] = nextFlows[idx-2];
} else if (idx > 0 &&
idx-1 < prevRow.length &&
prevRow[idx-1] === '/') {
// if ' _' - keep the '/' flow
// '/ '
nextFlows[idx] = flows[idx-1];
} else if (idx > 1 &&
idx-2 < prevRow.length &&
prevRow[idx-2] === '/') {
// if ' |_' - keep the '/' flow
// '/? '
nextFlows[idx] = flows[idx-2];
} else {
// There really shouldn't be another way of doing this - generate and wait for bug-reports...
nextFlows[idx] = genNewFlow();
}
break;
case '-': {
// This is: ------.
// /|\
// Find the end of the '-':
let originalIdx = idx;
for (;idx < currentRow.length && currentRow[idx] === '-'; idx++);
if (currentRow[idx] !== '.') {
// It really should end in a '.' but this one doesn't...
// Step back
idx--;
// Generate a new flow and await bug-reports...
nextFlows[idx] = genNewFlow();
// Assign all of the '-' to the same flow.
for (;originalIdx<idx;originalIdx++) {
nextFlows[originalIdx] = nextFlows[idx];
}
} else {
// We have a sensible ----.
// Choose our flow either /, |, or \
let flow;
if (idx-1 < prevRow.length && prevRow[idx-1] == '/') {
flow = flows[idx-1];
} else if (idx < prevRow.length && prevRow[idx] == '|') {
flow = flows[idx];
} else if (idx+1 < prevRow.length && prevRow[idx] == '\\') {
flow = flows[idx+1];
} else {
// Again unexpected so let's generate and wait the bug-report
flow = genNewFlow();
}
// Assign all of the rest of the ----. to this flow.
for (;originalIdx<idx+1;originalIdx++) {
nextFlows[originalIdx] = flow;
}
}
}
break;
case ' ':
// spaces don't get no flows
break;
default:
// Unexpected so let's generate a new flow and wait for bug-reports
flows[idx] = genNewFlow();
}
}
for (let idx = 0; idx < currentRow.length; idx++) {
x = config.unitSize * 0.5 * (idx + 1);
switch (currentRow[idx]) {
case '-':
case '.':
case '_':
drawLineRight(x - 0.5 *config.unitSize, y, nextFlows[idx].color);
break;
case '*':
drawNode(x, y, nextFlows[idx].color);
break;
case '|':
drawLineUp(x, y, nextFlows[idx].color);
break;
case '/':
drawLineOut(x - 0.5 *config.unitSize, y, nextFlows[idx].color);
break;
case '\\':
drawLineIn(x - 0.5 *config.unitSize, y, nextFlows[idx].color);
break;
case ' ':
break;
default:
console.error('Unknown symbol', currentRow[idx], nextFlows[idx] ? nextFlows[idx].color: '');
}
}
flows = nextFlows
}
};
init();
draw(graphList);
}
// @end-license
The above was an example not a complete fix.
zeripath
on 2 Jul 2020
On https://try.gitea.io/ahaha/glu/graph:
With new gitgraph.js:
ahttvy
on 3 Jul 2020
For anyone watching this I've just merged another rework of the GitGraph rendering that has moved its rendering to the server.
zeripath
on 6 Aug 2020
Was this page helpful?
0 / 5 - 0 ratings
Related issues
adpande
·
3Comments
lunny
·
3Comments
haytona
·
3Comments
lunny
·
3Comments
internalfx
·
3Comments