Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions backend/core/src/sources/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use async_trait::async_trait;
use git2::{Direction, RemoteCallbacks};
use sea_orm::EntityTrait;
use std::sync::Arc;
use tracing::{debug, info, instrument, warn};
use tracing::{debug, info, instrument};

// ── ProjectGitContext ─────────────────────────────────────────────────────────

Expand Down Expand Up @@ -79,7 +79,7 @@ impl<'a> ProjectGitContext<'a> {
let ssh_creds = self.ssh_creds.clone();
let branch_owned = branch.map(|b| b.to_owned());

let remote_hash = match tokio::task::spawn_blocking(move || {
let remote_hash = tokio::task::spawn_blocking(move || {
let branch_ref = branch_owned.as_deref();
if let Some((private_key, public_key)) = ssh_creds {
ls_remote_head(&url, Some(&private_key), Some(&public_key), branch_ref)
Expand All @@ -90,13 +90,7 @@ impl<'a> ProjectGitContext<'a> {
.await
.map_err(|e| SourceError::GitExecution {
error: e.to_string(),
})? {
Ok(hash) => hash,
Err(e) => {
warn!(error = %e, "Failed to get remote HEAD ref, will retry next cycle");
return Ok((false, vec![]));
}
};
})??;

let remote_hash_str = vec_to_hex(&remote_hash);
debug!(remote_hash = %remote_hash_str, "Retrieved remote hash");
Expand Down
48 changes: 48 additions & 0 deletions backend/core/tests/git_remote.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* SPDX-FileCopyrightText: 2026 Wavelens GmbH <info@wavelens.io>
*
* SPDX-License-Identifier: AGPL-3.0-only
*/

//! Tests for `check_project_updates` error propagation.
//!
//! Issue #280: manual `POST /evaluate` used to swallow git fetch failures
//! (DNS, connection refused, …) and bubble up a generic 500. The fix
//! propagates the `SourceError` to the caller so the web layer can return
//! a 4xx with a useful message.

extern crate core as gradient_core;

use entity::project;
use gradient_core::sources::check_project_updates;
use sea_orm::{DatabaseBackend, MockDatabase};
use test_support::state::test_state;

/// `git://` URLs use the pure-Rust pkt-line path, so `TcpStream::connect`
/// to an unbound loopback port returns "connection refused" without
/// touching the network beyond loopback. Before the fix, this case was
/// hidden behind `Ok((false, vec![]))`.
#[test]
fn check_project_updates_propagates_unreachable_remote_error() {
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.unwrap();
rt.block_on(async {
let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection();
let state = test_state(db);
let project = project::Model {
repository: "git://127.0.0.1:1/nonexistent.git".into(),
force_evaluation: true,
..Default::default()
};

let result = check_project_updates(state, &project, None).await;

assert!(
result.is_err(),
"expected Err for unreachable git:// URL, got {:?}",
result
);
});
}
15 changes: 7 additions & 8 deletions backend/web/src/endpoints/projects/evaluations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use super::{
use crate::access::{Caller, ProjectAccess, has_permission, is_org_member, load_project};
use crate::authorization::{MaybeApiKey, MaybeUser};
use crate::endpoints::content_type_for_filename;
use crate::error::{WebError, WebResult};
use crate::error::{ErrorCode, WebError, WebResult};
use crate::helpers::{OptionExt, ok_json};
use crate::permissions::Permission;
use axum::extract::{Path, Query, State};
Expand Down Expand Up @@ -213,13 +213,12 @@ pub async fn post_project_evaluate(
let (_has_updates, commit_hash) =
check_project_updates(Arc::clone(&state), &project_for_check, None)
.await
.map_err(|e| anyhow::anyhow!(e))?;

if commit_hash.is_empty() {
return Err(WebError::internal(
"Failed to fetch repository state".to_string(),
));
}
.map_err(|e| {
WebError::bad_request_with(
ErrorCode::REPOSITORY_UNREACHABLE,
format!("Failed to fetch repository state: {}", e),
)
})?;

let eval = gradient_core::ci::trigger_evaluation(
&state.web_db,
Expand Down
23 changes: 12 additions & 11 deletions backend/web/src/endpoints/projects/management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use super::ProjectResponse;
use crate::access::{Caller, OrgAccess, ProjectAccess, has_permission, load_org, load_project};
use crate::audit::{RequestInfo, events, record as audit_record};
use crate::authorization::{MaybeApiKey, MaybeUser};
use crate::error::{WebError, WebResult};
use crate::error::{ErrorCode, WebError, WebResult};
use crate::helpers::{OptionExt, ok_json};
use crate::permissions::Permission;
use axum::extract::{Path, Query, State};
Expand Down Expand Up @@ -629,18 +629,19 @@ pub async fn post_project_check_repository(

let (_has_updates, remote_hash) = check_project_updates(Arc::clone(&state), &project, None)
.await
.map_err(|e| anyhow::anyhow!(e))?;
.map_err(|e| {
WebError::bad_request_with(
ErrorCode::REPOSITORY_UNREACHABLE,
format!("Failed to check repository: {}", e),
)
})?;

if !remote_hash.is_empty() {
let res = BaseResponse {
error: false,
message: vec_to_hex(&remote_hash),
};
let res = BaseResponse {
error: false,
message: vec_to_hex(&remote_hash),
};

Ok(Json(res))
} else {
Err(WebError::internal("Failed to check repository".to_string()))
}
Ok(Json(res))
}

pub async fn post_project_transfer(
Expand Down
1 change: 1 addition & 0 deletions backend/web/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ impl ErrorCode {
pub const INVALID_OAUTH_CODE: Self = Self("invalid_oauth_code");
pub const OAUTH_DISABLED: Self = Self("oauth_disabled");
pub const REGISTRATION_DISABLED: Self = Self("registration_disabled");
pub const REPOSITORY_UNREACHABLE: Self = Self("repository_unreachable");

// 401 Unauthorized
pub const UNAUTHORIZED: Self = Self("unauthorized");
Expand Down
30 changes: 27 additions & 3 deletions docs/gradient-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1744,6 +1744,9 @@ paths:
Tests whether Gradient can clone the project's repository using the
organization's SSH key. Requires Admin or Write role; rejects
state-managed projects.

Returns `400` with `code: "repository_unreachable"` and the underlying
git error when the repository cannot be fetched.
operationId: checkProjectRepository
responses:
'200':
Expand All @@ -1753,7 +1756,13 @@ paths:
schema:
$ref: '#/components/schemas/StringResponse'
'400':
$ref: '#/components/responses/BadRequest'
description: >-
Bad request - invalid input or repository unreachable
(`code: "repository_unreachable"`).
content:
application/json:
schema:
$ref: '#/components/schemas/StringResponse'
'401':
$ref: '#/components/responses/Unauthorized'
'403':
Expand All @@ -1768,7 +1777,12 @@ paths:
post:
tags: [projects]
summary: Trigger evaluation
description: Queues a new evaluation for the project. Returns the new evaluation UUID.
description: >-
Queues a new evaluation for the project. Returns the new evaluation UUID.

Returns `400` with `code: "repository_unreachable"` when the repository
cannot be fetched (DNS failure, authentication failure, unreachable host,
bad URL, …) - the message includes the underlying git error.
operationId: triggerEvaluation
responses:
'200':
Expand All @@ -1781,7 +1795,17 @@ paths:
error: false
message: "3fa85f64-5717-4562-b3fc-2c963f66afa6"
'400':
$ref: '#/components/responses/BadRequest'
description: >-
Bad request - invalid input, evaluation already in progress, or
repository unreachable (`code: "repository_unreachable"`).
content:
application/json:
schema:
$ref: '#/components/schemas/StringResponse'
example:
error: true
code: repository_unreachable
message: 'Failed to fetch repository state: Git command failed: failed to resolve address for github: Name or service not known'
'401':
$ref: '#/components/responses/Unauthorized'
'404':
Expand Down
30 changes: 30 additions & 0 deletions docs/src/tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -2658,3 +2658,33 @@ Run with: `pnpm --dir frontend exec ng test --watch=false`

- `backend/core/src/db/admin_tasks.rs` — DB helper unit tests: insert/find/mark transitions, unique-violation detection, startup recovery `mark_all_active_failed`.
- `backend/cache/src/cacher/deep_gc.rs` — sweep unit tests: blob pass removes orphan blob, blob pass purges zombie row, log pass removes orphan log, `DeepGcReport` serialises with snake_case keys.

## Evaluation start - surface repository errors (issue #280)

`POST /projects/{org}/{p}/evaluate` and `POST /projects/{org}/{p}/check-repository`
used to swallow git fetch failures (DNS, connection refused, auth) inside
`core::sources::git::check_for_updates` and bubble up a generic 500 with no
actionable detail. The fix propagates the `SourceError` to the web layer, which
maps it to `400 Bad Request` with `code: "repository_unreachable"` and the
underlying git error message. The frontend surfaces that message in an inline
banner under the project header instead of failing silently.

### Backend

Run with: `cargo test -p core --test git_remote`

- `check_project_updates_propagates_unreachable_remote_error` — `git://127.0.0.1:1/…`
triggers an immediate connection-refused; the helper now returns `Err(SourceError)`
instead of `Ok((false, vec![]))`. Locks in the propagation guarantee that the
endpoint relies on for its 4xx mapping.

### Frontend

Run with: `pnpm --dir frontend exec ng test --watch=false`

- `project-detail.component.spec.ts → 'shows an inline error banner when
startEvaluation fails'` — mocks `ProjectsService.startEvaluation` to throw;
asserts the `.evaluation-error` banner renders the underlying message.
- `project-detail.component.spec.ts → 'clears the error banner when the user
retries'` — calling `dismissError()` resets `errorMessage()` to `null` and the
banner disappears.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ <h1>{{ proj.display_name }}</h1>
[routerLink]="['/organization', orgName, 'project', projectName, 'metrics']"
></button>
</div>
@if (errorMessage(); as msg) {
<div class="evaluation-error" role="alert">
<span class="material-symbols-outlined">error</span>
<span class="evaluation-error__text">{{ msg }}</span>
<button
type="button"
class="evaluation-error__dismiss"
aria-label="Dismiss error"
(click)="dismissError()"
>
<span class="material-symbols-outlined">close</span>
</button>
</div>
}
<div class="project-meta">
<span class="meta-item">
<span class="material-symbols-outlined">link</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,43 @@
}
}

.evaluation-error {
flex-basis: 100%;
display: flex;
align-items: center;
gap: $spacing-sm;
padding: $spacing-sm $spacing-md;
background-color: rgba($color-danger, 0.1);
border: 1px solid $color-danger;
border-radius: $border-radius-sm;
color: $color-danger;
font-size: $font-size-sm;

.material-symbols-outlined {
font-size: 18px;
flex-shrink: 0;
}

.evaluation-error__text {
flex: 1 1 auto;
word-break: break-word;
}

.evaluation-error__dismiss {
background: none;
border: 0;
color: inherit;
cursor: pointer;
padding: 0;
display: inline-flex;
align-items: center;

.material-symbols-outlined {
font-size: 18px;
}
}
}

.stats-grid {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing';
import { ActivatedRoute, convertToParamMap, provideRouter } from '@angular/router';
import { provideHttpClient } from '@angular/common/http';
import { provideHttpClientTesting } from '@angular/common/http/testing';
import { of } from 'rxjs';
import { of, throwError } from 'rxjs';
import { ProjectDetailComponent } from './project-detail.component';
import { ProjectsService } from '@core/services/projects.service';
import { OrganizationsService } from '@core/services/organizations.service';
Expand Down Expand Up @@ -122,3 +122,57 @@ describe('ProjectDetailComponent - access gating', () => {
expect(restartBtn!.disabled).toBe(false);
});
});

// ── Issue #280: surface evaluation start failures to the user ────────────────

function setupWithStartError(message: string): ComponentFixture<ProjectDetailComponent> {
const access: AccessState = { managed: false, canEdit: true, canTrigger: true };
TestBed.configureTestingModule({
imports: [ProjectDetailComponent],
providers: [
provideRouter([]),
provideHttpClient(),
provideHttpClientTesting(),
{ provide: ActivatedRoute, useValue: activatedRouteStub(access) },
{
provide: ProjectsService,
useValue: {
getProject: () => of(projectFor(access)),
getEntryPoints: () => of([]),
startEvaluation: () => throwError(() => new Error(message)),
restartFailedBuilds: () => throwError(() => new Error(message)),
},
},
{ provide: OrganizationsService, useValue: { getOrganization: () => of({ display_name: 'Acme' }) } },
{ provide: AuthService, useValue: { isAuthenticated: () => true } },
],
});
const fixture = TestBed.createComponent(ProjectDetailComponent);
fixture.detectChanges();
return fixture;
}

describe('ProjectDetailComponent - error surfacing (issue #280)', () => {
it('shows an inline error banner when startEvaluation fails', () => {
const fixture = setupWithStartError('Failed to fetch repository state: connection refused');
fixture.componentInstance.startEvaluation();
fixture.detectChanges();

const banner = fixture.nativeElement.querySelector('.evaluation-error') as HTMLElement | null;
expect(banner).not.toBeNull();
expect(banner!.textContent).toContain('Failed to fetch repository state');
});

it('clears the error banner when the user retries', () => {
const fixture = setupWithStartError('Failed to fetch repository state: connection refused');
const component = fixture.componentInstance;
component.startEvaluation();
fixture.detectChanges();
expect(component.errorMessage()).not.toBeNull();

component.dismissError();
fixture.detectChanges();
expect(component.errorMessage()).toBeNull();
expect(fixture.nativeElement.querySelector('.evaluation-error')).toBeNull();
});
});
Loading
Loading