fix: stop dropping blank update fields, and recover errors from binary requests - #47
Merged
Merged
Conversation
…y requests Two problems that both ended with the workflow being told less than the server knew. **A blank value in an update collection was dropped in silence.** Template, Label and Automation Rule each skipped a blank `name`, `body` or `replyText` because the server marks those fields non-empty and forwarding one is a guaranteed 400. Skipping avoids the round trip but loses the user: a blank supplied alongside another field produced a successful partial update, so the name looked updated and was not, and a blank supplied on its own produced "at least one field must be provided" while a field was plainly sitting in the collection. There is no reading under which a blank means something here, so it is now refused by name, with a message that says how to leave the field unchanged. The three handlers share one helper rather than repeating the policy, since it has to stay consistent across them. A blank `header` or `footer` is untouched: those carry no such validator, so clearing them is legitimate and still works. **An error from a binary request lost its body.** Operations that download media ask for an arraybuffer, so when the server answers with an error its JSON body arrives as raw bytes and the explanation never reaches the workflow. Message > Get Media is where this bites: its 404 covers five different situations, and without the body the workflow sees only the status code. The body is now decoded back into the error before it is thrown, so the server's own message survives. The decoding is guarded throughout and never applied to a non-binary request, which is passed through untouched.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Two cases where the node told the workflow less than the server knew.
A blank value in an update collection was dropped in silence
Template Update, Label Create or Update, and Automation Rule Update each skipped a blank
name,bodyorreplyText. The reason was sound: the server marks those fields non-empty, so forwarding a blank is a guaranteed 400 and skipping it saves a round trip. The result was not.There is no reading under which a blank means something for these fields, so it is now refused by name, with a message saying how to leave the field unchanged. The three handlers share one helper (
optionalNonBlank) rather than repeating the policy, because it has to stay consistent across them.A blank
headerorfooteron a template is deliberately untouched: those carry no non-empty validator, so clearing them is legitimate and still works. There is a test holding that line.An error from a binary request lost its body
Operations that download media ask for an
arraybuffer, so when the server answers with an error its JSON body arrives as raw bytes and the explanation never reaches the workflow.Message > Get Media is where this matters. Its 404 covers five distinct situations (no media on the message, download disabled, payload over the storage cap, a URL-based send whose bytes are never stored, or the message not being in this gateway's history), and without the body the workflow sees only a status code.
The body is now decoded back onto the error before it is thrown, so the server's message survives into the
NodeApiError. The decoding is fully guarded, so decorating an error can never itself throw, and it is applied only to binary requests: a normal JSON failure is passed through untouched, which a test asserts.Verification
npm run build,npm run lint, 424 tests (up from 419), and the n8n Creator Portal scanner all pass; the committeddist/matches source.Both fixes were negative-controlled: with the fix reverted in the build output, the new tests fail, and they pass again once it is restored. The request-body conformance check (every body the node can build, validated against the server DTOs for undeclared fields, missing required fields, wrong types and bad enums) still reports zero problems.