Skip to content

Commit 9ad35d8

Browse files
rakduttarakdutta1
rakdutta
and
rakdutta1
authored
Improved Error for Add Prompt and edit Prompt for Issue #361 and Issue #591 (#629)
* issue 363 prompt Signed-off-by: RAKHI DUTTA <[email protected]> * issue 363 prompt Signed-off-by: RAKHI DUTTA <[email protected]> * admin.js Signed-off-by: RAKHI DUTTA <[email protected]> * doctest Signed-off-by: RAKHI DUTTA <[email protected]> * issue 561 Signed-off-by: RAKHI DUTTA <[email protected]> * add prompt Signed-off-by: RAKHI DUTTA <[email protected]> * edit prompt Signed-off-by: RAKHI DUTTA <[email protected]> * edit prompt Signed-off-by: RAKHI DUTTA <[email protected]> * 363 prompt Signed-off-by: RAKHI DUTTA <[email protected]> * test case Signed-off-by: RAKHI DUTTA <[email protected]> * test case Signed-off-by: RAKHI DUTTA <[email protected]> * test cases Signed-off-by: RAKHI DUTTA <[email protected]> * issue 363 Signed-off-by: RAKHI DUTTA <[email protected]> --------- Signed-off-by: RAKHI DUTTA <[email protected]> Co-authored-by: RAKHI DUTTA <[email protected]>
1 parent 91977e1 commit 9ad35d8

File tree

10 files changed

+284
-83
lines changed

10 files changed

+284
-83
lines changed

‎mcpgateway/admin.py

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ async def admin_add_server(request: Request, db: Session = Depends(get_db), user
436436
return JSONResponse(content={"message": str(ex), "success": False}, status_code=422)
437437

438438
except Exception as ex:
439+
logger.info(f"error,{ex}")
439440
if isinstance(ex, ServerError):
440441
# Custom server logic error — 500 Internal Server Error makes sense
441442
return JSONResponse(content={"message": str(ex), "success": False}, status_code=500)
@@ -3163,26 +3164,39 @@ async def admin_add_prompt(request: Request, db: Session = Depends(get_db), user
31633164
>>>
31643165
>>> async def test_admin_add_prompt():
31653166
... response = await admin_add_prompt(mock_request, mock_db, mock_user)
3166-
... return isinstance(response, RedirectResponse) and response.status_code == 303
3167+
... return isinstance(response, JSONResponse) and response.status_code == 200 and response.body == b'{"message":"Prompt registered successfully!","success":true}'
31673168
>>>
31683169
>>> asyncio.run(test_admin_add_prompt())
31693170
True
3171+
31703172
>>> prompt_service.register_prompt = original_register_prompt
31713173
"""
31723174
logger.debug(f"User {user} is adding a new prompt")
31733175
form = await request.form()
3174-
args_json = form.get("arguments") or "[]"
3175-
arguments = json.loads(args_json)
3176-
prompt = PromptCreate(
3177-
name=form["name"],
3178-
description=form.get("description"),
3179-
template=form["template"],
3180-
arguments=arguments,
3181-
)
3182-
await prompt_service.register_prompt(db, prompt)
3183-
3184-
root_path = request.scope.get("root_path", "")
3185-
return RedirectResponse(f"{root_path}/admin#prompts", status_code=303)
3176+
try:
3177+
args_json = form.get("arguments") or "[]"
3178+
arguments = json.loads(args_json)
3179+
prompt = PromptCreate(
3180+
name=form["name"],
3181+
description=form.get("description"),
3182+
template=form["template"],
3183+
arguments=arguments,
3184+
)
3185+
await prompt_service.register_prompt(db, prompt)
3186+
return JSONResponse(
3187+
content={"message": "Prompt registered successfully!", "success": True},
3188+
status_code=200,
3189+
)
3190+
except Exception as ex:
3191+
if isinstance(ex, ValidationError):
3192+
logger.error(f"ValidationError in admin_add_prompt: {ErrorFormatter.format_validation_error(ex)}")
3193+
return JSONResponse(content=ErrorFormatter.format_validation_error(ex), status_code=422)
3194+
if isinstance(ex, IntegrityError):
3195+
error_message = ErrorFormatter.format_database_error(ex)
3196+
logger.error(f"IntegrityError in admin_add_prompt: {error_message}")
3197+
return JSONResponse(status_code=409, content=error_message)
3198+
logger.error(f"Error in admin_add_prompt: {ex}")
3199+
return JSONResponse(content={"message": str(ex), "success": False}, status_code=500)
31863200

31873201

31883202
@admin_router.post("/prompts/{name}/edit")
@@ -3235,7 +3249,7 @@ async def admin_edit_prompt(
32353249
>>>
32363250
>>> async def test_admin_edit_prompt():
32373251
... response = await admin_edit_prompt(prompt_name, mock_request, mock_db, mock_user)
3238-
... return isinstance(response, RedirectResponse) and response.status_code == 303
3252+
... return isinstance(response, JSONResponse) and response.status_code == 200 and response.body == b'{"message":"Prompt update successfully!","success":true}'
32393253
>>>
32403254
>>> asyncio.run(test_admin_edit_prompt())
32413255
True
@@ -3261,20 +3275,35 @@ async def admin_edit_prompt(
32613275
form = await request.form()
32623276
args_json = form.get("arguments") or "[]"
32633277
arguments = json.loads(args_json)
3264-
prompt = PromptUpdate(
3265-
name=form["name"],
3266-
description=form.get("description"),
3267-
template=form["template"],
3268-
arguments=arguments,
3269-
)
3270-
await prompt_service.update_prompt(db, name, prompt)
3278+
try:
3279+
prompt = PromptUpdate(
3280+
name=form["name"],
3281+
description=form.get("description"),
3282+
template=form["template"],
3283+
arguments=arguments,
3284+
)
3285+
await prompt_service.update_prompt(db, name, prompt)
32713286

3272-
root_path = request.scope.get("root_path", "")
3273-
is_inactive_checked = form.get("is_inactive_checked", "false")
3287+
root_path = request.scope.get("root_path", "")
3288+
is_inactive_checked = form.get("is_inactive_checked", "false")
32743289

3275-
if is_inactive_checked.lower() == "true":
3276-
return RedirectResponse(f"{root_path}/admin/?include_inactive=true#prompts", status_code=303)
3277-
return RedirectResponse(f"{root_path}/admin#prompts", status_code=303)
3290+
if is_inactive_checked.lower() == "true":
3291+
return RedirectResponse(f"{root_path}/admin/?include_inactive=true#prompts", status_code=303)
3292+
# return RedirectResponse(f"{root_path}/admin#prompts", status_code=303)
3293+
return JSONResponse(
3294+
content={"message": "Prompt update successfully!", "success": True},
3295+
status_code=200,
3296+
)
3297+
except Exception as ex:
3298+
if isinstance(ex, ValidationError):
3299+
logger.error(f"ValidationError in admin_add_prompt: {ErrorFormatter.format_validation_error(ex)}")
3300+
return JSONResponse(content=ErrorFormatter.format_validation_error(ex), status_code=422)
3301+
if isinstance(ex, IntegrityError):
3302+
error_message = ErrorFormatter.format_database_error(ex)
3303+
logger.error(f"IntegrityError in admin_add_prompt: {error_message}")
3304+
return JSONResponse(status_code=409, content=error_message)
3305+
logger.error(f"Error in admin_add_prompt: {ex}")
3306+
return JSONResponse(content={"message": str(ex), "success": False}, status_code=500)
32783307

32793308

32803309
@admin_router.post("/prompts/{name}/delete")

‎mcpgateway/config.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,7 @@ def validate_database(self) -> None:
495495
validation_dangerous_html_pattern: str = (
496496
r"<(script|iframe|object|embed|link|meta|base|form|img|svg|video|audio|source|track|area|map|canvas|applet|frame|frameset|html|head|body|style)\b|</*(script|iframe|object|embed|link|meta|base|form|img|svg|video|audio|source|track|area|map|canvas|applet|frame|frameset|html|head|body|style)>"
497497
)
498+
498499
validation_dangerous_js_pattern: str = r"(?i)(?:^|\s|[\"'`<>=])(javascript:|vbscript:|data:\s*[^,]*[;\s]*(javascript|vbscript)|\bon[a-z]+\s*=|<\s*script\b)"
499500

500501
validation_allowed_url_schemes: List[str] = ["http://", "https://", "ws://", "wss://"]

‎mcpgateway/services/prompt_service.py

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ async def register_prompt(self, db: Session, prompt: PromptCreate) -> PromptRead
197197
Created prompt information
198198
199199
Raises:
200-
PromptNameConflictError: If prompt name already exists
200+
IntegrityError: If a database integrity error occurs.
201201
PromptError: For other prompt registration errors
202202
203203
Examples:
@@ -219,16 +219,6 @@ async def register_prompt(self, db: Session, prompt: PromptCreate) -> PromptRead
219219
... pass
220220
"""
221221
try:
222-
# Check for name conflicts (both active and inactive)
223-
existing_prompt = db.execute(select(DbPrompt).where(DbPrompt.name == prompt.name)).scalar_one_or_none()
224-
225-
if existing_prompt:
226-
raise PromptNameConflictError(
227-
prompt.name,
228-
is_active=existing_prompt.is_active,
229-
prompt_id=existing_prompt.id,
230-
)
231-
232222
# Validate template syntax
233223
self._validate_template(prompt.template)
234224

@@ -267,9 +257,9 @@ async def register_prompt(self, db: Session, prompt: PromptCreate) -> PromptRead
267257
prompt_dict = self._convert_db_prompt(db_prompt)
268258
return PromptRead.model_validate(prompt_dict)
269259

270-
except IntegrityError:
271-
db.rollback()
272-
raise PromptError(f"Prompt already exists: {prompt.name}")
260+
except IntegrityError as ie:
261+
logger.error(f"IntegrityErrors in group: {ie}")
262+
raise ie
273263
except Exception as e:
274264
db.rollback()
275265
raise PromptError(f"Failed to register prompt: {str(e)}")
@@ -429,7 +419,7 @@ async def update_prompt(self, db: Session, name: str, prompt_update: PromptUpdat
429419
430420
Raises:
431421
PromptNotFoundError: If the prompt is not found
432-
PromptNameConflictError: If the new prompt name already exists
422+
IntegrityError: If a database integrity error occurs.
433423
PromptError: For other update errors
434424
435425
Examples:
@@ -457,15 +447,6 @@ async def update_prompt(self, db: Session, name: str, prompt_update: PromptUpdat
457447

458448
raise PromptNotFoundError(f"Prompt not found: {name}")
459449

460-
if prompt_update.name is not None and prompt_update.name != prompt.name:
461-
existing_prompt = db.execute(select(DbPrompt).where(DbPrompt.name == prompt_update.name).where(DbPrompt.id != prompt.id)).scalar_one_or_none()
462-
if existing_prompt:
463-
raise PromptNameConflictError(
464-
prompt_update.name,
465-
is_active=existing_prompt.is_active,
466-
prompt_id=existing_prompt.id,
467-
)
468-
469450
if prompt_update.name is not None:
470451
prompt.name = prompt_update.name
471452
if prompt_update.description is not None:
@@ -494,6 +475,10 @@ async def update_prompt(self, db: Session, name: str, prompt_update: PromptUpdat
494475
await self._notify_prompt_updated(prompt)
495476
return PromptRead.model_validate(self._convert_db_prompt(prompt))
496477

478+
except IntegrityError as ie:
479+
db.rollback()
480+
logger.error(f"IntegrityErrors in group: {ie}")
481+
raise ie
497482
except Exception as e:
498483
db.rollback()
499484
raise PromptError(f"Failed to update prompt: {str(e)}")

‎mcpgateway/static/admin.js

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4311,7 +4311,6 @@ async function handleGatewayFormSubmit(e) {
43114311
}
43124312
}
43134313
}
4314-
43154314
async function handleResourceFormSubmit(e) {
43164315
e.preventDefault();
43174316
const form = e.target;
@@ -4378,6 +4377,111 @@ async function handleResourceFormSubmit(e) {
43784377
}
43794378
}
43804379

4380+
async function handlePromptFormSubmit(e) {
4381+
e.preventDefault();
4382+
const form = e.target;
4383+
const formData = new FormData(form);
4384+
const status = safeGetElement("status-prompts");
4385+
const loading = safeGetElement("add-prompts-loading");
4386+
try {
4387+
// Validate inputs
4388+
const name = formData.get("name");
4389+
const nameValidation = validateInputName(name, "prompt");
4390+
4391+
if (!nameValidation.valid) {
4392+
showErrorMessage(nameValidation.error);
4393+
return;
4394+
}
4395+
4396+
if (loading) {
4397+
loading.style.display = "block";
4398+
}
4399+
if (status) {
4400+
status.textContent = "";
4401+
status.classList.remove("error-status");
4402+
}
4403+
4404+
const isInactiveCheckedBool = isInactiveChecked("prompts");
4405+
formData.append("is_inactive_checked", isInactiveCheckedBool);
4406+
4407+
const response = await fetchWithTimeout(
4408+
`${window.ROOT_PATH}/admin/prompts`,
4409+
{
4410+
method: "POST",
4411+
body: formData,
4412+
},
4413+
);
4414+
4415+
const result = await response.json();
4416+
if (!result.success) {
4417+
throw new Error(result.message || "An error occurred");
4418+
}
4419+
// Only redirect on success
4420+
const redirectUrl = isInactiveCheckedBool
4421+
? `${window.ROOT_PATH}/admin?include_inactive=true#prompts`
4422+
: `${window.ROOT_PATH}/admin#prompts`;
4423+
window.location.href = redirectUrl;
4424+
} catch (error) {
4425+
console.error("Error:", error);
4426+
if (status) {
4427+
status.textContent = error.message || "An error occurred!";
4428+
status.classList.add("error-status");
4429+
}
4430+
showErrorMessage(error.message);
4431+
} finally {
4432+
// location.reload();
4433+
if (loading) {
4434+
loading.style.display = "none";
4435+
}
4436+
}
4437+
}
4438+
4439+
async function handleEditPromptFormSubmit(e) {
4440+
e.preventDefault();
4441+
const form = e.target;
4442+
const formData = new FormData(form);
4443+
4444+
try {
4445+
// Validate inputs
4446+
const name = formData.get("name");
4447+
const nameValidation = validateInputName(name, "prompt");
4448+
if (!nameValidation.valid) {
4449+
showErrorMessage(nameValidation.error);
4450+
return;
4451+
}
4452+
4453+
// Save CodeMirror editors' contents if present
4454+
if (window.promptToolHeadersEditor) {
4455+
window.promptToolHeadersEditor.save();
4456+
}
4457+
if (window.promptToolSchemaEditor) {
4458+
window.promptToolSchemaEditor.save();
4459+
}
4460+
4461+
const isInactiveCheckedBool = isInactiveChecked("prompts");
4462+
formData.append("is_inactive_checked", isInactiveCheckedBool);
4463+
4464+
// Submit via fetch
4465+
const response = await fetch(form.action, {
4466+
method: "POST",
4467+
body: formData,
4468+
});
4469+
4470+
const result = await response.json();
4471+
if (!result.success) {
4472+
throw new Error(result.message || "An error occurred");
4473+
}
4474+
// Only redirect on success
4475+
const redirectUrl = isInactiveCheckedBool
4476+
? `${window.ROOT_PATH}/admin?include_inactive=true#prompts`
4477+
: `${window.ROOT_PATH}/admin#prompts`;
4478+
window.location.href = redirectUrl;
4479+
} catch (error) {
4480+
console.error("Error:", error);
4481+
showErrorMessage(error.message);
4482+
}
4483+
}
4484+
43814485
async function handleServerFormSubmit(e) {
43824486
e.preventDefault();
43834487

@@ -5079,6 +5183,21 @@ function setupFormHandlers() {
50795183
resourceForm.addEventListener("submit", handleResourceFormSubmit);
50805184
}
50815185

5186+
const promptForm = safeGetElement("add-prompt-form");
5187+
if (promptForm) {
5188+
promptForm.addEventListener("submit", handlePromptFormSubmit);
5189+
}
5190+
5191+
const editPromptForm = safeGetElement("edit-prompt-form");
5192+
if (editPromptForm) {
5193+
editPromptForm.addEventListener("submit", handleEditPromptFormSubmit);
5194+
editPromptForm.addEventListener("click", () => {
5195+
if (getComputedStyle(editPromptForm).display !== "none") {
5196+
refreshEditors();
5197+
}
5198+
});
5199+
}
5200+
50825201
const toolForm = safeGetElement("add-tool-form");
50835202
if (toolForm) {
50845203
toolForm.addEventListener("submit", handleToolFormSubmit);

‎mcpgateway/templates/admin.html

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,9 +1464,7 @@ <h3 class="text-lg font-bold mb-4 dark:text-gray-200">
14641464
<form
14651465
method="POST"
14661466
action="{{ root_path }}/admin/prompts"
1467-
id="add-prompt-form"
1468-
onsubmit="return handleToggleSubmit(event, 'prompts')"
1469-
>
1467+
id="add-prompt-form">
14701468
<div class="grid grid-cols-1 gap-6">
14711469
<div>
14721470
<label
@@ -1515,6 +1513,10 @@ <h3 class="text-lg font-bold mb-4 dark:text-gray-200">
15151513
></textarea>
15161514
</div>
15171515
</div>
1516+
<div id="add-prompts-loading" style="display: none">
1517+
<div class="spinner"></div>
1518+
</div>
1519+
<div id="status-prompts"></div>
15181520
<div class="mt-6">
15191521
<button
15201522
type="submit"
@@ -2708,7 +2710,7 @@ <h3 class="text-lg font-medium text-gray-900 dark:text-gray-100">
27082710
name="template"
27092711
id="edit-prompt-template"
27102712
class="mt-1 block w-full h-48 rounded-md border border-gray-300 shadow-sm focus:border-indigo-500 focus:ring-indigo-500"
2711-
required
2713+
27122714
></textarea>
27132715
</div>
27142716
<div>

0 commit comments

Comments
 (0)