Improve error handling and refactor geometry generation

[chrisreddington]

Enhance error handling throughout the codebase and refactor the geometry generation logic for better clarity and performance.

This includes introducing a new error type for general errors and ensuring proper validation of API responses.

Additionally, streamline concurrency handling in geometry-related functions.

[Copilot]

Pull request overview

This PR strengthens error categorization/validation and refactors STL geometry generation to be more deterministic and safer under concurrency, while updating tests to cover the new behaviors.

Changes:

• Add a GeneralError type and update errors.Wrap to avoid mislabeling unknown errors as STL.
• Validate GitHub API join-date responses using time.Time.IsZero() and expand client tests.
• Refactor model geometry generation to use buffered channels (avoiding goroutine leaks on early returns), stable component ordering, and add multi-year input validation; update generator tests accordingly.

Show a summary per file

File	Description
internal/stl/geometry/geometry.go	Simplifies year spacing constants and uses YearOffset for multi-year column placement.
internal/stl/generator.go	Adds multi-year input validation, refactors concurrent geometry orchestration for deterministic ordering and buffered channels, and routes contribution geometry generation through geometry with error returns.
internal/stl/generator_test.go	Updates tests for buffered channels and new CreateContributionGeometry error-return signature; adds a GridSize overflow case.
internal/logger/logger_test.go	Adds unit coverage for Warning log behavior across log levels.
internal/github/client.go	Validates CreatedAt from the API with IsZero() before deriving the join year.
internal/github/client_test.go	Adds a “zero join date” test case and cleans up unused time-based fixtures.
internal/errors/errors.go	Introduces GeneralError and uses it as the default type in Wrap for non-Skyline errors.
internal/errors/errors_test.go	Updates expectations for Wrap default error prefix/type.
cmd/skyline/skyline.go	Simplifies ASCII printing logic in multi-year generation.
cmd/root.go	Returns errors from --web path instead of printing and exiting directly.

Copilot's findings

Comments suppressed due to low confidence (4)

internal/stl/generator.go:224

• With the refactor removing the WaitGroup from generateText, it may be a good moment to align error propagation with generateColumnsForYearRange: if text generation fails and the warning log call also fails, the current code reports logErr and drops the original text-generation error. Consider returning the original geometry error (treat logging as best-effort) so failures remain diagnosable.

// generateText creates 3D text geometry for the model
func generateText(username string, startYear int, endYear int, dims modelDimensions, ch chan<- geometryResult) {
	embossedYear := fmt.Sprintf("%d", endYear)

	// If start year and end year are the same, only show one year

internal/stl/generator.go:212

• In the failure path, if logging the warning fails, the function returns logErr and drops the original geometry error (err). That makes the caller see a logging failure instead of the real root cause. Prefer treating logging failures as secondary (best-effort) and return the original geometry error (optionally wrapping/annotating it) for consistency with the comment in generateColumnsForYearRange.

This issue also appears in the following locations of the same file:

• line 220
• line 244

	if err != nil {
		if logErr := logger.GetLogger().Warning("Failed to generate base geometry: %v. Continuing without base.", err); logErr != nil {
			ch <- geometryResult{triangles: []types.Triangle{}, err: logErr}
			return
		}

internal/stl/generator.go:250

• If logo generation fails and logger.Warning also fails, the returned error is logErr rather than the original logo-generation error. This hides the root cause and makes failures harder to diagnose; consider returning the original err (optionally wrapping it) and treating logging errors as secondary.

	logoTriangles, err := geometry.GenerateImageGeometry(dims.innerWidth, geometry.BaseHeight)
	if err != nil {
		// Log warning and continue without logo instead of failing
		if logErr := logger.GetLogger().Warning("Failed to generate logo geometry: %v. Continuing without logo.", err); logErr != nil {
			ch <- geometryResult{triangles: []types.Triangle{}, err: logErr}
			return
		}

internal/stl/geometry/geometry.go:71

• Now that CreateContributionGeometry returns an error, consider wrapping CreateColumn failures with context (yearIndex/weekIdx/dayIdx) so callers/logs can pinpoint which cell caused the geometry failure. Right now the raw error bubbles up without location information.

	baseYOffset := 2*CellSize + float64(yearIndex)*YearOffset

	for weekIdx, week := range contributions {
		for dayIdx, day := range week {
			if day.ContributionCount > 0 {

• Files reviewed: 10/10 changed files
• Comments generated: 2

[liamchampton]

lgtm