Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
6 changes: 4 additions & 2 deletions backend/infrahub/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ async def login_user(
auth_method=AuthMethod.PASSWORD,
)
await service.event.send(event=event)
except Exception as ex:
# Login event emission is best-effort telemetry; it must never fail a successful login
except Exception as ex: # noqa: BLE001

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this was needed in the first place. There would be an exception here if the event itself was malformed i.e. we didn't properly create the event. That should never happen as we control the data used as input. When sending the events we'd be sending them to an internal queue which is then later picked up. I don't think this ever blocks the current code path even if we fail to send the event to Prefect. In some other cases we do send things like this as a background task that gets executed after the response is returned the user which could be a better option. The same would be true for oauth2 and oicd, as this PR isn't really changing the behavior this part could be done as a follow up job.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 platform-health-agent — thanks, and agreed with your own conclusion that this is a follow-up rather than something for this PR: the try/except around the event emission already existed, and this PR adds only the comment + # noqa: BLE001, so behaviour is unchanged here.

To make sure the follow-up captures your point accurately: the suggestion is that the defensive catch is unnecessary in the first place (event construction is fully under our control, and emission goes to an internal queue that cannot block the login/logout response), so the better shape is to drop the try/except — or move emission to a background task that runs after the response is returned — rather than keep a suppressed broad catch. Same treatment for api/oauth2.py and api/oidc.py, which carry the identical pattern.

I have not changed anything here. If you would like this tracked as an Engineering Inbox card (all three files, "remove/relocate best-effort event-emission catches in auth endpoints"), say so and I will file it with your reasoning quoted.

log.warning(f"Failed to emit login event for account_id={auth_result.account_id}: {str(ex)}")
return auth_result.token

Expand Down Expand Up @@ -113,7 +114,8 @@ async def logout(
session_id=session_id,
)
await service.event.send(event=event)
except Exception as ex:
# Logout event emission is best-effort telemetry; it must never fail a successful logout
except Exception as ex: # noqa: BLE001
log.warning(f"Failed to emit logout event for account_id={user_session.account_id}: {str(ex)}")

delete_response_cookies(response=response)
Expand Down
3 changes: 2 additions & 1 deletion backend/infrahub/api/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ async def token(
identity_source=provider_name,
)
await service.event.send(event=event)
except Exception as ex:
# Login event emission is best-effort telemetry; it must never fail a successful OAuth2 login
except Exception as ex: # noqa: BLE001
log.warning(f"Failed to emit OAuth2 login event for account_id={auth_result.account_id}: {str(ex)}")

return models.UserTokenWithUrl(
Expand Down
3 changes: 2 additions & 1 deletion backend/infrahub/api/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ async def token(
identity_source=provider_name,
)
await service.event.send(event=event)
except Exception as ex:
# Login event emission is best-effort telemetry; it must never fail a successful OIDC login
except Exception as ex: # noqa: BLE001
log.warning(f"Failed to emit OIDC login event for account_id={auth_result.account_id}: {str(ex)}")

return models.UserTokenWithUrl(
Expand Down
3 changes: 2 additions & 1 deletion backend/infrahub/artifacts/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ async def create(model: CheckArtifactCreate) -> ValidatorConclusion:
check_message = "Artifact rendered successfully"
conclusion = ValidatorConclusion.SUCCESS

except Exception as exc:
# Check boundary: any render failure must be recorded as a failed artifact check, not crash the flow
except Exception as exc: # noqa: BLE001
artifact.status.value = "Error"
await artifact.save()
severity = "critical"
Expand Down
12 changes: 8 additions & 4 deletions backend/infrahub/auth/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,8 @@ async def validate_jwt_access_token(token: str) -> AccountSession:
session_id = payload["session_id"]
except jwt.ExpiredSignatureError:
raise AuthorizationError("Expired Signature") from None
except Exception:
# Fail closed: any undecodable or malformed token must map to a 401 auth error, never a 500
except Exception: # noqa: BLE001
raise AuthorizationError("Invalid token") from None

if payload["type"] == "access":
Expand All @@ -555,7 +556,8 @@ async def validate_jwt_refresh_token(db: InfrahubDatabase, token: str) -> models
session_id = payload["session_id"]
except jwt.ExpiredSignatureError:
raise AuthorizationError("Expired Signature") from None
except Exception:
# Fail closed: any undecodable or malformed refresh token must map to a 401, never a 500
except Exception: # noqa: BLE001
raise AuthorizationError("Invalid token") from None

await validate_active_account(db=db, account_id=str(account_id))
Expand Down Expand Up @@ -665,7 +667,8 @@ def safe_get_response_body(response: httpx.Response, raise_error_on_empty_body:
# Try to parse as JSON first
try:
return response.json()
except Exception as json_error:
# Providers may return non-JSON bodies: fall back to text or fail closed with GatewayError (502)
except Exception as json_error: # noqa: BLE001
try:
# Try to get as text
text_body = response.text
Expand All @@ -676,7 +679,8 @@ def safe_get_response_body(response: httpx.Response, raise_error_on_empty_body:
status_code=response.status_code,
)
raise GatewayError(message="Authentication provider returned an empty response") from json_error
except Exception:
# If the body cannot be read at all, fail closed with GatewayError (502) rather than a 500
except Exception: # noqa: BLE001
log.error(
"Unable to read response body from authentication provider",
url=str(response.url),
Expand Down
6 changes: 4 additions & 2 deletions backend/infrahub/cli/upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ async def validate_prerequisites(db: InfrahubDatabase) -> bool:
except DatabaseError as exc:
console.log(f"{ERROR_BADGE} Database prerequisite check failed: {exc}")
return False
except Exception as exc:
# CLI prerequisite boundary: report any failure as an unreachable database and abort cleanly
except Exception as exc: # noqa: BLE001
console.log(f"{ERROR_BADGE} Database is unreachable: {exc}")
console.log(
" Verify that the database is running and that the connection settings in your configuration file are correct."
Expand Down Expand Up @@ -241,7 +242,8 @@ async def _upgrade_check(db: InfrahubDatabase, root_node_graph_version: int) ->
console.log(" Schema has differences, update required")
else:
console.log(" Up to date, nothing to do")
except Exception as exc:
# Best-effort dry-run report: a failed schema probe is reported inline and the remaining checks still run
except Exception as exc: # noqa: BLE001
console.log(f" Unable to check: {exc}")

console.log("\nBranches:")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
index_manager = IndexManagerNeo4j(db=db)
index_manager.init(nodes=[INDEX_TO_DELETE], rels=[])
await index_manager.drop()
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
result.errors.append(str(exc))
return result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
duplicate_relationships_cleanup_query = await DeleteDuplicateRelationships.init(db=db)
await duplicate_relationships_cleanup_query.execute(db=db)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
migration_result.errors.append(str(exc))
return migration_result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
index_manager = IndexManagerNeo4j(db=db)
index_manager.init(nodes=[INDEX_TO_DELETE], rels=[])
await index_manager.drop()
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
result.errors.append(str(exc))
return result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
execution_result = await migration.execute(migration_input=migration_input, branch=default_branch)
result.errors.extend(execution_result.errors)
progress.update(update_task, advance=1)
except Exception as exc:
# First failing sub-migration is recorded as a result error and aborts the remaining steps
except Exception as exc: # noqa: BLE001
result.errors.append(str(exc))
return result

Expand Down Expand Up @@ -165,7 +166,8 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch:
execution_result = await migration.execute(migration_input=migration_input, branch=branch)
result.errors.extend(execution_result.errors)
progress.update(update_task, advance=1)
except Exception as exc:
# First failing sub-migration is recorded as a result error and aborts the remaining steps
except Exception as exc: # noqa: BLE001
result.errors.append(str(exc))
return result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
update_task=update_task,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
return MigrationResult()

Expand Down Expand Up @@ -511,6 +512,7 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch:
at=at,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
return MigrationResult()
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
update_task=update_task,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
return MigrationResult()

Expand Down Expand Up @@ -160,6 +161,7 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch:
at=at,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
return MigrationResult()
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ async def _do_one_schema_all(
async def execute(self, migration_input: MigrationInput) -> MigrationResult:
try:
return await self._do_execute(migration_input=migration_input)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])

async def _do_execute(self, migration_input: MigrationInput) -> MigrationResult:
Expand Down Expand Up @@ -193,7 +194,8 @@ async def _do_execute(self, migration_input: MigrationInput) -> MigrationResult:
execution_result = await migration.execute(migration_input=migration_input, branch=global_branch)
result.errors.extend(execution_result.errors)
progress.update(update_task, advance=1)
except Exception as exc:
# First failing sub-migration is recorded as a result error and aborts the remaining steps
except Exception as exc: # noqa: BLE001
result.errors.append(str(exc))
return result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
update_task=backfill_task,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
return MigrationResult()

Expand Down Expand Up @@ -462,6 +463,7 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch:
at=at,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
return MigrationResult()
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ async def _compute_values_for_batch(
value = await self._compute_display_label(db=db, schema=schema, node=node, console=console)
if value is not None:
dl_values[node_uuid] = value
except Exception as exc:
# Best-effort per-node recompute: record the failure, skip this node, keep fixing the rest
except Exception as exc: # noqa: BLE001
console.print(f" Skipping display_label for {node_uuid} ({kind}): {exc}")
errors.append(f"display_label compute failed for {node_uuid} ({kind}): {exc}")

Expand All @@ -244,7 +245,8 @@ async def _compute_values_for_batch(
value = await self._compute_hfid(db=db, schema=schema, node=node, console=console)
if value is not None:
hfid_values[node_uuid] = value
except Exception as exc:
# Best-effort per-node recompute: record the failure, skip this node, keep fixing the rest
except Exception as exc: # noqa: BLE001
console.print(f" Skipping human_friendly_id for {node_uuid} ({kind}): {exc}")
errors.append(f"human_friendly_id compute failed for {node_uuid} ({kind}): {exc}")

Expand Down Expand Up @@ -378,7 +380,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
else:
console.print("No nodes with bad values found on global branch")

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
result.errors.append(str(exc))

return result
Expand Down Expand Up @@ -417,5 +420,6 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch:
progress=progress,
progress_task=task,
)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
update_task=update_task,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}"
return MigrationResult(errors=[error_msg])

Expand All @@ -470,7 +471,8 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch:
await self._compute_object_permission_display_labels(
db=db, branch=branch, attribute_schema=display_label_attribute_schema, console=console
)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}"
return MigrationResult(errors=[error_msg])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ async def _process_templates(
)
await query.execute(db=db)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}"
return MigrationResult(errors=[error_msg])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ async def _process_templates(
migration_input=migration_input,
)

except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}"
return MigrationResult(errors=[error_msg])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:

await self._update_schema_parameters(db=dbt, pool_id_map=pool_id_map, console=console)

except Exception as exc:
# Failures become MigrationResult errors so the runner reports them instead of crashing.
# CAVEAT: returning here from inside the transaction context means that context sees no
# exception and commits, so a mid-loop failure can persist partial consolidation.
# Re-raising (or moving this handler outside the transaction) is a separate follow-up.
except Exception as exc: # noqa: BLE001
Comment thread
saltas888 marked this conversation as resolved.
error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}"
return MigrationResult(errors=[error_msg])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ async def _run(
display_label_attribute_schema=display_label_attribute_schema,
at=at,
)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc) or f"{type(exc).__name__}: {repr(exc)}"])

return MigrationResult()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ async def _run(
display_label_attribute_schema=display_label_attribute_schema,
at=at,
)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc) or f"{type(exc).__name__}: {repr(exc)}"])

return MigrationResult()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
try:
await self._normalize_hfid_values(db=db)
await self._index_hfid_values(db=db)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])

return MigrationResult()
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
await self._bootstrap_core_ip_pool_generic(db=dbt, at=schema_root_at, user_id=user_id)
await self._append_inherit_from_for_all_pools(db=dbt)
await self._rewrite_resource_relationship_attributes(db=dbt)
except Exception as exc:
# Failures become MigrationResult errors so the runner reports them instead of crashing
except Exception as exc: # noqa: BLE001
error_msg = str(exc) or f"{type(exc).__name__}: {exc!r}"
return MigrationResult(errors=[error_msg])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult:
db = migration_input.db
try:
await self._normalize_hfid_values(db=db)
except Exception as exc:
# Migration contract: failures become MigrationResult errors; the runner reports them and halts
except Exception as exc: # noqa: BLE001
return MigrationResult(errors=[str(exc)])

return MigrationResult()
Loading
Loading