Fix exception when passing model: null to the api/pdf endpoint#29
Fix exception when passing model: null to the api/pdf endpoint#29marcominerva merged 3 commits intomasterfrom
Conversation
Co-authored-by: marcominerva <3522534+marcominerva@users.noreply.github.com>
- Updated `JsonDocumentExtensions.cs` for comment clarity. - Enhanced null checks and error handling in `PdfService.cs` for template engine and time zone management. - Allowed nullable model parameters in `HandlebarsTemplateEngine.cs`, `RazorTemplateEngine.cs`, and `ScribanTemplateEngine.cs`. - Updated `ITemplateEngine.cs` interface for consistency with nullable model parameters. - Renamed constant in `ScribanTemplateEngine.cs` for improved clarity.
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes an exception that occurred when passing model: null to the /api/pdf endpoint. The main issue was that the JSON deserialization logic in JsonDocumentExtensions.ConvertElement only handled Object and Array types, throwing an InvalidOperationException for null values.
Key changes made:
- Modified
ConvertElementmethod to delegate non-Object/Array cases to the existingConvertValuemethod instead of throwing exceptions - Updated template engine interfaces and implementations to accept nullable model parameters
- Simplified PDF generation logic to handle null models consistently
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| JsonDocumentExtensions.cs | Fixed the core issue by delegating null/primitive value handling to existing ConvertValue method |
| ITemplateEngine.cs | Updated interface to accept nullable model parameter |
| ScribanTemplateEngine.cs | Updated implementation to handle nullable models and fixed constant name |
| RazorTemplateEngine.cs | Updated to accept nullable models and removed extra whitespace |
| HandlebarsTemplateEngine.cs | Updated to accept nullable models |
| PdfService.cs | Simplified logic to use null-conditional operator for model conversion |
Comments suppressed due to low confidence (1)
src/PdfSmith.BusinessLayer/Templating/ScribanTemplateEngine.cs:13
- The constant name change from 'date_time_zone' to 'datetime_withzone' appears unrelated to the null model fix. This naming change should be separated into its own PR or better justified in the current context.
private const string DateTimeZonePlaceholder = "datetime_withzone";
| string? content; | ||
| try | ||
| { | ||
| var model = request.Model?.ToExpandoObject(timeZoneInfo); |
There was a problem hiding this comment.
[nitpick] The logic has been simplified but the control flow could be clearer. Consider handling the null model case explicitly rather than relying on the null-conditional operator, especially since the original code had separate paths for null vs non-null models.
| var model = request.Model?.ToExpandoObject(timeZoneInfo); | |
| object? model; | |
| if (request.Model is null) | |
| { | |
| model = null; | |
| } | |
| else | |
| { | |
| model = request.Model.ToExpandoObject(timeZoneInfo); | |
| } |
The API was throwing an
InvalidOperationExceptionwhen a user passedmodel: nullin the JSON request body to the/api/pdfendpoint. This occurred because theJsonDocumentExtensions.ConvertElementmethod only handledJsonValueKind.ObjectandJsonValueKind.Arraytypes, but notJsonValueKind.Nullat the root level.Issue
When sending a request like this:
{ "template": "My Template!", "model": null, "options": { "pageSize": "A4", "orientation": "Portrait", "margin": null }, "templateEngine": "razor", "fileName": null }The following exception was thrown:
Solution
Modified the
ConvertElementmethod to delegate non-Object/Array cases to the existingConvertValuemethod, which already properly handles allJsonValueKindvalues includingNull. This is a minimal change that reuses existing logic and maintains backward compatibility.Changes
ConvertElementto returnobject?instead of throwing for unsupported value kindsToExpandoObjectreturn type toobject?to properly reflect nullable resultsConvertValuefor handling all non-Object/Array casesThe fix ensures that
model: nullis properly handled by returningnullinstead of throwing an exception, while preserving all existing functionality for object and array models.Fixes #28.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.