Fix: Provide clear error when filename is missing on /data endpoint (Fixes #220) #1705
+153
−87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hyperledger#1528) Signed-off-by: Deekshit S <deekshit1403@gmail.com>
hyperledger#220) Signed-off-by: Deekshit S <deekshit1403@gmail.com>
Deekshit014
force-pushed
the
fix/misleading-error-220
branch
from
6cd4c37 to
4cc3525
Compare
EnriqueL8
requested changes
Dec 2, 2025
Contributor
EnriqueL8
left a comment
EnriqueL8
left a comment
There was a problem hiding this comment.
Thanks @Deekshit014 for the PR - a few comments and changes needed. Would be great to add a test for the new validation
| @@ -0,0 +1,7 @@ | |||
| { | |||
| @@ -0,0 +1,4 @@ | |||
| info: | |||
Comment on lines
+62
to
+65
| // Check for file upload | ||
| if r.Part == nil || r.Part.FileName() == "" { | ||
| return nil, i18n.NewError(r.Req.Context(), coremsgs.MsgMissingFileUpload) | ||
| } |
Contributor
There was a problem hiding this comment.
For clarity this is what changed
Comment on lines
+28
to
+36
| // @Summary Update an identity | ||
| // @ID patchUpdateIdentity | ||
| // @Tags identities | ||
| // @Accept json | ||
| // @Produce json | ||
| // @Param iid path string true "Identity ID" <-- THE CRITICAL FIX | ||
| // @Param body body core.IdentityUpdateDTO true "Identity update details" | ||
| // @Success 202 {object} core.Identity | ||
| // @Router /identities/{iid} [patch] |
Contributor
There was a problem hiding this comment.
These remind me of sprint boot tags, where did you get these from?
| "github.com/hyperledger/firefly/pkg/core" | ||
| ) | ||
|
|
||
| // @Summary Get identity by ID |
Contributor
There was a problem hiding this comment.
ditto it would be significant decisions to bring in a DSL for describing APIs since we already have within ffapi the ability to describe the API and transform it to an OpenAPI document
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Please include a summary of the changes here and why we need those changes. And also let us know which issue is fixed.
This Pull Request resolves Issue #220.
The issue was that when a developer attempted to upload data to the
/dataendpoint using themultipart/form-datacontent type but omitted the requiredfilename, the API returned a generic or confusing validation error.The change introduces an explicit check at the start of the form upload handler to ensure the file part and filename are present, returning a clear, user-friendly error message (
MsgFilenameRequired) instead of letting the request fail later with a misleading error.Fixes #220
Types of changes
Please make sure to follow these points
Screenshots (If Applicable)
Other Information
The fix was applied in
internal/apiserver/route_post_data.gowithin theCoreFormUploadHandler. I inserted the check forr.Part == nil || r.Part.FileName == ""at the start of the handler to ensure the error is returned at the point of validation, leading to better developer experience.