fix(api): stop reporting a misconfigured server as an unreachable platform

`POST /api/auth/login` answered 502 platform_unreachable — "Could not reach
Loyaly. Check your connection and try again." — for a fault that is entirely
ours and that no connection check can fix.

`buildUrl()` is where LOYALY_API_BASE is read, and it was called INSIDE the
try block whose catch turns a failed fetch into UpstreamError(0, 'network').
So the config guard threw, the catch swallowed it, and "nobody set
LOYALY_API_BASE" arrived at the route indistinguishable from "the platform is
down". Measured on the pre-fix code, all of these produced the identical
UpstreamError(status=0, code=network):

  LOYALY_API_BASE unset
  LOYALY_API_BASE=https://platform.loyaly.ai   (the known-wrong host)
  LOYALY_API_BASE=not-a-url
  nothing listening on the far end             (the only real network failure)

The message survived, so the truth was reachable, but only by reading the
prose of an error the code had already classified as a network fault — and
the login route had by then replaced it with advice about the user's wifi.

ConfigError now exists for this, buildUrl is resolved before the try in both
upstreamRequest and upstreamRaw, and callers branch on it: 500 misconfigured,
not 502 unreachable. 500 is the honest status — a bad gateway says the thing
upstream is unwell, and this server has not got as far as having an upstream.
Verified after the change: the four config faults raise ConfigError, and a
dead port still raises UpstreamError(0, 'network').

The detail is logged, never returned. It names an environment variable and the
hosts this console accepts, which belongs in the Dokploy log pane rather than
in an anonymous sign-in form's response body. `[loyaly] configuration error:`
is now the line to grep for.

Also fixes failJson labelling a 5xx as `unauthorized` in the envelope: the
code is derived from the status now, so a misconfigured server can no longer
tell a browser the password was wrong. That one costs somebody a password
reset for a fault they cannot see.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-09-17 19:35:54 +05:30
parent a40afb9e7a
commit 3dc0bba6f4
4 changed files with 112 additions and 8 deletions

View File

@@ -1,7 +1,7 @@
import {NextResponse} from 'next/server'; import {NextResponse} from 'next/server';
import type {NextRequest} from 'next/server'; import type {NextRequest} from 'next/server';
import {authApi} from '@/services/api/authApi'; import {authApi} from '@/services/api/authApi';
import {UpstreamError} from '@/services/api/apiClient'; import {ConfigError, UpstreamError} from '@/services/api/apiClient';
import { import {
LOGIN_ERROR_PARAM, LOGIN_ERROR_PARAM,
type LoginErrorCode, type LoginErrorCode,
@@ -71,9 +71,19 @@ async function parse(req: NextRequest): Promise<ParsedLogin> {
}; };
} }
/**
* The envelope `code` is derived from the STATUS, not passed in, so the two can
* never disagree. `reason` carries the specific login code alongside it.
*
* 5xx maps to 'internal' rather than falling through to 'unauthorized': a
* misconfigured server telling the browser the credentials were rejected is a
* lie that costs somebody a password reset.
*/
function failJson(code: LoginErrorCode, message: string, status: number) { function failJson(code: LoginErrorCode, message: string, status: number) {
const envelopeCode =
status >= 500 ? 'internal' : status === 429 ? 'bad_request' : 'unauthorized';
return Response.json( return Response.json(
{error: {code: status === 429 ? 'bad_request' : 'unauthorized', message}, field: 'form', reason: code}, {error: {code: envelopeCode, message}, field: 'form', reason: code},
{status, headers: {'cache-control': 'no-store'}}, {status, headers: {'cache-control': 'no-store'}},
); );
} }
@@ -96,6 +106,35 @@ export async function POST(req: NextRequest) {
try { try {
bundle = await authApi.login(email, password); bundle = await authApi.login(email, password);
} catch (err) { } catch (err) {
/*
* A misconfigured server, before anything about the credentials matters.
*
* Checked FIRST and kept out of the unreachable branch below. Both used to
* land on 502 platform_unreachable — "Could not reach Loyaly, check your
* connection" — for a fault that is entirely ours and that no amount of
* checking a connection will fix. 500 is the honest status: this server
* cannot serve, as opposed to an upstream that is unwell.
*
* The detail names an environment variable, so it is logged rather than
* returned. An anonymous sign-in form is the last place to publish which
* hosts a deployment accepts.
*/
if (err instanceof ConfigError) {
console.error('[loyaly] configuration error:', err.message);
const code: LoginErrorCode = 'misconfigured';
if (isForm) {
return NextResponse.redirect(
new URL(`/login?${LOGIN_ERROR_PARAM}=${code}`, req.url),
303,
);
}
return failJson(
code,
'Sign-in is unavailable right now. Please contact support.',
500,
);
}
const up = err instanceof UpstreamError ? err : null; const up = err instanceof UpstreamError ? err : null;
// The platform answers wrong-password and no-such-account identically, on // The platform answers wrong-password and no-such-account identically, on

View File

@@ -37,6 +37,20 @@ const LOGIN_ERRORS: Record<string, LoginError> = {
field: 'form', field: 'form',
message: 'Could not reach Loyaly. Check your connection and try again.', message: 'Could not reach Loyaly. Check your connection and try again.',
}, },
/**
* The SERVER is misconfigured — LOYALY_API_BASE is missing or points
* somewhere that is not the platform. Separate from platform_unreachable
* because the remedies have nothing in common: "check your connection" is
* actively wrong advice for a fault that no user can do anything about, and
* it had people checking their wifi while an env var sat unset.
*
* Says nothing about which variable. The operator detail goes to the server
* log; this is what the person at the form reads.
*/
misconfigured: {
field: 'form',
message: 'Sign-in is unavailable right now. Please contact support.',
},
/** /**
* Correct credentials for a PLATFORM ADMIN — an account with no company. * Correct credentials for a PLATFORM ADMIN — an account with no company.
* Every surface in this console is tenant-scoped, so there is nothing here * Every surface in this console is tenant-scoped, so there is nothing here

View File

@@ -84,8 +84,30 @@ const KNOWN_WRONG_HOSTS: Record<string, string> = {
'it call its own origin', 'it call its own origin',
}; };
function configError(detail: string): Error { /**
return new Error( * A deployment is misconfigured. NOT a network failure, and the distinction is
* the whole point of the class existing.
*
* Both used to arrive at a caller as `UpstreamError(0, 'network')`, because
* `buildUrl` — which resolves the variable — was called INSIDE the try block
* that turns a failed fetch into that error. So "nobody set LOYALY_API_BASE"
* and "the platform is down" were indistinguishable at every call site, and
* the login route reported both as 502 platform_unreachable. That sent people
* to check DNS, egress and the platform's health for a fault whose fix is one
* line in an env file.
*
* Callers translate this to a 500: a server that cannot be configured is not a
* bad gateway, and no retry or status page will help.
*/
export class ConfigError extends Error {
constructor(message: string) {
super(message);
this.name = 'ConfigError';
}
}
function configError(detail: string): ConfigError {
return new ConfigError(
`LOYALY_API_BASE is invalid: ${detail}. ` + `LOYALY_API_BASE is invalid: ${detail}. ` +
`Set it to ${PRODUCTION_API_ORIGIN} in production, or ${DEV_API_BASE} locally.`, `Set it to ${PRODUCTION_API_ORIGIN} in production, or ${DEV_API_BASE} locally.`,
); );
@@ -147,7 +169,7 @@ function resolveBase(): string {
} }
if (isProduction) { if (isProduction) {
throw new Error( throw new ConfigError(
'LOYALY_API_BASE is required in production — refusing to guess the ' + 'LOYALY_API_BASE is required in production — refusing to guess the ' +
'Loyaly platform host. Set it to https://mcp.loyaly.ai.', 'Loyaly platform host. Set it to https://mcp.loyaly.ai.',
); );
@@ -258,9 +280,14 @@ export async function upstreamRequest<T>(req: UpstreamRequest): Promise<T> {
const hasBody = body !== undefined && method !== 'GET'; const hasBody = body !== undefined && method !== 'GET';
if (hasBody) headers['content-type'] = 'application/json'; if (hasBody) headers['content-type'] = 'application/json';
// Resolved OUTSIDE the try on purpose: this is where LOYALY_API_BASE is read,
// and a ConfigError thrown here must reach the caller as itself rather than
// being caught below and relabelled "could not reach the platform".
const url = buildUrl(path, query);
let res: Response; let res: Response;
try { try {
res = await fetch(buildUrl(path, query), { res = await fetch(url, {
method, method,
headers, headers,
body: hasBody ? JSON.stringify(body) : undefined, body: hasBody ? JSON.stringify(body) : undefined,
@@ -298,7 +325,9 @@ export async function upstreamRaw(req: UpstreamRequest): Promise<Response> {
const headers: Record<string, string> = {}; const headers: Record<string, string> = {};
if (accessToken) headers.authorization = `Bearer ${accessToken}`; if (accessToken) headers.authorization = `Bearer ${accessToken}`;
const res = await fetch(buildUrl(path, query), { // Outside any catch, for the same reason as upstreamRequest above.
const url = buildUrl(path, query);
const res = await fetch(url, {
method, method,
headers, headers,
signal, signal,

View File

@@ -1,6 +1,6 @@
import 'server-only'; import 'server-only';
import type {NextRequest} from 'next/server'; import type {NextRequest} from 'next/server';
import {UpstreamError} from '@/services/api/apiClient'; import {ConfigError, UpstreamError} from '@/services/api/apiClient';
import {NoSessionError, withUpstream} from '@/features/auth/services/upstreamSession'; import {NoSessionError, withUpstream} from '@/features/auth/services/upstreamSession';
import {ok, parseQuery, type Query} from '@/shared/services/apiRoute'; import {ok, parseQuery, type Query} from '@/shared/services/apiRoute';
import type {ApiErrorCode} from '@/shared/types/api'; import type {ApiErrorCode} from '@/shared/types/api';
@@ -46,6 +46,28 @@ export function failureFrom(err: unknown): UpstreamFailure {
return {status: 401, code: 'unauthorized', message: err.message}; return {status: 401, code: 'unauthorized', message: err.message};
} }
/**
* A misconfigured deployment. 500, never 502 — a bad gateway says "the thing
* upstream is unwell, try later", and this server has not got as far as
* having an upstream. Retrying and checking the platform's health both waste
* the reader's time.
*
* The detail is LOGGED, never returned. It names an environment variable and
* the hosts this console does and does not accept, which is operator
* information and not something to hand an anonymous browser. The log line is
* the only copy, and it is what shows up in `docker logs` / the Dokploy log
* pane the moment the first request comes in.
*/
if (err instanceof ConfigError) {
console.error('[loyaly] configuration error:', err.message);
return {
status: 500,
code: 'internal',
message: 'This console is not configured correctly. Please contact support.',
reason: 'misconfigured',
};
}
if (err instanceof UpstreamError) { if (err instanceof UpstreamError) {
// 501 is "the feature is off for this deployment", not a fault. It is // 501 is "the feature is off for this deployment", not a fault. It is
// surfaced as its own reason so a panel can say "not available here" // surfaced as its own reason so a panel can say "not available here"