Skip to content

[Bug]: Optional stored procedure parameters generate the error "Missing required procedure parameters" #1748

Closed
#2346
Closed
[Bug]: Optional stored procedure parameters generate the error "Missing required procedure parameters"#1748
#2346
Assignees
seantleonard
Labels
bugSomething isn't workingconfigchanges related to configgraphqlreststored-procedure
Milestone
1.3

Description

@yorek
Davide Mauri (yorek)
opened on Sep 26, 2023

What happened?

I have a stored procedure like the following:

create procedure dbo.stp_Dummy
@mandatoryParam int,
@optionalParam int = 10
as
select 
    @mandatoryParam as mandatoryParam,
    @optionalParam as optionalParam
go

for which the @optionalParam is, as the name implies, optional.

If I configure DAB to use that stored procedure using the following configuration, omitting the optionalParam (since it is optional):

"Dummy": {
    "source": {
      "type": "stored-procedure",
      "object": "dbo.stp_Dummy",
      "parameters": {
        "mandatoryParam": 10
      }
    },
    "graphql": {
      "operation": "query"
    },
    "permissions": [
      {
        "role": "anonymous",
        "actions": [
          "execute"
        ]
      }
    ]
  }

I get the following error during the startup:

 Azure.DataApiBuilder.Service.Exceptions.DataApiBuilderException: Invalid request. Missing required procedure parameters: optionalParam for entity: Dummy

Version

Microsoft.DataApiBuilder 0.8.52+c69925060e1942d28515b9c4b89ea24832da0c7c

What database are you using?

Azure SQL

What hosting model are you using?

Local (including CLI)

Which API approach are you accessing DAB through?

REST, GraphQL

Relevant log output

Information: Microsoft.DataApiBuilder 0.8.52+c69925060e1942d28515b9c4b89ea24832da0c7c
Information: Config not provided. Trying to get default config based on DAB_ENVIRONMENT...
Information: Environment variable DAB_ENVIRONMENT is (null)
Loading config file from dab-config.json.
Information: Loaded config file: dab-config.json
Information: Setting default minimum LogLevel: Debug for Development mode.
Starting the runtime engine...
Loading config file from dab-config.json.
info: Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager[63]
      User profile is available. Using 'C:\Users\damauri\AppData\Local\ASP.NET\DataProtection-Keys' as key repository and Windows DPAPI to encrypt keys at rest.
info: Azure.DataApiBuilder.Core.Services.ISqlMetadataProvider[0]
      Dummy path: /api/Dummy
dbug: Azure.DataApiBuilder.Core.Resolvers.IQueryExecutor[0]
      Executing query:
SELECT name as result_field_name, TYPE_NAME(system_type_id) as result_type, is_nullable FROM sys.dm_exec_describe_first_result_set_for_object (OBJECT_ID('dbo.stp_Dummy'), 0) WHERE is_hidden is not NULL AND is_hidden = 0
dbug: Azure.DataApiBuilder.Core.Services.ISqlMetadataProvider[0]
      Logging primary key information for entity: Dummy.
info: Azure.DataApiBuilder.Core.Configurations.RuntimeConfigValidator[0]
      Validating Relationship Section in Config...
fail: Azure.DataApiBuilder.Service.Startup[0]
      Unable to complete runtime initialization. Refer to exception for error details.
      Azure.DataApiBuilder.Service.Exceptions.DataApiBuilderException: Invalid request. Missing required procedure parameters: optionalParam for entity: Dummy
         at Azure.DataApiBuilder.Core.Configurations.RuntimeConfigValidator.ValidateStoredProceduresInConfig(RuntimeConfig runtimeConfig, ISqlMetadataProvider sqlMetadataProvider)
         at Azure.DataApiBuilder.Service.Startup.PerformOnConfigChangeAsync(IApplicationBuilder app)
fail: Azure.DataApiBuilder.Service.Startup[0]
      Exiting the runtime engine...
crit: Microsoft.AspNetCore.Hosting.Diagnostics[6]
      Application startup exception
      System.ApplicationException: Could not initialize the engine with the runtime config file: dab-config.json
         at Azure.DataApiBuilder.Service.Startup.Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeConfigProvider runtimeConfigProvider)
         at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
         at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
         at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
         at Microsoft.AspNetCore.Hosting.ConfigureBuilder.<>c__DisplayClass4_0.<Build>b__0(IApplicationBuilder builder)
         at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.<>c__DisplayClass15_0.<UseStartup>b__1(IApplicationBuilder app)  
         at Microsoft.AspNetCore.Mvc.Filters.MiddlewareFilterBuilderStartupFilter.<>c__DisplayClass0_0.<Configure>g__MiddlewareFilterBuilder|0(IApplicationBuilder builder)
         at Microsoft.AspNetCore.HostFilteringStartupFilter.<>c__DisplayClass0_0.<Configure>b__0(IApplicationBuilder app)       
         at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
Unable to launch the runtime due to: System.ApplicationException: Could not initialize the engine with the runtime config file: dab-config.json
   at Azure.DataApiBuilder.Service.Startup.Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeConfigProvider runtimeConfigProvider)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
   at Microsoft.AspNetCore.Hosting.ConfigureBuilder.<>c__DisplayClass4_0.<Build>b__0(IApplicationBuilder builder)
   at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.<>c__DisplayClass15_0.<UseStartup>b__1(IApplicationBuilder app)        
   at Microsoft.AspNetCore.Mvc.Filters.MiddlewareFilterBuilderStartupFilter.<>c__DisplayClass0_0.<Configure>g__MiddlewareFilterBuilder|0(IApplicationBuilder builder)
   at Microsoft.AspNetCore.HostFilteringStartupFilter.<>c__DisplayClass0_0.<Configure>b__0(IApplicationBuilder app)
   at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.Run(IHost host)
   at Azure.DataApiBuilder.Service.Program.StartEngine(String[] args)
Error: Failed to start the engine.

Code of Conduct

  • I agree to follow this project's Code of Conduct

Activity

  1. Davide Mauri (yorek)
    added
    bugSomething isn't working
    triageissues to be triaged
    on Sep 26, 2023

  2. rohkhann commented on Sep 26, 2023

    @rohkhann
    rohkhann
    on Sep 26, 2023
    Contributor

    this is related to this: #989. There is a validation error that is done on the runtimeconfig where we check that number of params on stored proc should match number in runtimeconfig, which is incorrect. We had reported this during our sync ups. I have the fix for this and will check it in soon.

  3. rohkhann
    self-assigned this
    on Sep 26, 2023
  4. Sean Leonard (seantleonard)
    added this to Data API builderon Feb 8, 2024
  5. github-project-automation
    moved this to Todo in Data API builderon Feb 8, 2024

  6. seantleonard commented on Feb 8, 2024

    @seantleonard
    Sean Leonard (seantleonard)
    on Feb 8, 2024
    Contributor

    rohkhann and Abhishek Kumar (@abhishekkumams) can you check if #1847 does indeed resolve this issue too? if so, please link the pr item and close this.

  7. Sean Leonard (seantleonard)
    removed
    triageissues to be triaged
    on Feb 8, 2024

  8. seantleonard commented on Mar 6, 2024

    @seantleonard
    Sean Leonard (seantleonard)
    on Mar 6, 2024
    Contributor

    rohkhann to confirm whether this is resolved.

  9. simonsabin commented on Mar 22, 2024

    @simonsabin
    Simon Sabin (simonsabin)
    on Mar 22, 2024
    Collaborator

    We're hitting this issue

  10. seantleonard commented on Mar 26, 2024

    @seantleonard
    Sean Leonard (seantleonard)
    on Mar 26, 2024
    Contributor

    Simon Sabin (@simonsabin), which version of DAB are you using?

  11. simonsabin commented on Mar 26, 2024

    @simonsabin
    Simon Sabin (simonsabin)
    on Mar 26, 2024 via email
    Collaborator

  12. rohkhann commented on Mar 28, 2024

    @rohkhann
    rohkhann
    on Mar 28, 2024
    Contributor

    Now, if you have parameters in your stored proc, you dont need to duplicate those parameters on the config file. Thats what this fix handles: #1847. However, we still dont have the capability to detect an optional parameter if it is not specified in the call to the stored proc.

  13. simonsabin commented on Mar 28, 2024

    @simonsabin
    Simon Sabin (simonsabin)
    on Mar 28, 2024
    Collaborator

    Thats a massive problem as there is no way to deploy the change without breaking the system. Which ever is deployed first, DAB or DB, the result is DAB fails to run until both are deployed

  14. Sean Leonard (seantleonard)
    added this to the 1.2rc milestone on Apr 29, 2024

  15. 11 remaining items

  16. seantleonard commented on Jul 25, 2024

    @seantleonard
    Sean Leonard (seantleonard)
    on Jul 25, 2024
    Contributor

    Benjamin Talmard (@Benjiiim), agreed with your and Simon Sabin (@simonsabin)'s finding. Working on a fix that incorporate both of your experiences encountering errors.

    There isn't a straightforward method to determine whether a stored procedure has a default valued parameter and what that value is without parsing the object definition which is error prone.

    Instead, the change I'm making essentially defers error handling to the database. In this case, error 201:

    "201", // Procedure or function '%.*ls' expects parameter '%.*ls', which was not supplied.

    SELECT message_id AS Error,
    severity AS Severity,
    [Event Logged] = CASE is_event_logged
        WHEN 0 THEN 'No' ELSE 'Yes'
        END,
    [text] AS [Description]
FROM sys.messages
WHERE language_id = 1033 
and message_id = 201

    Consequently, DAB will not fail startup or terminate a request when DAB doesn't detect an optional parameter in the request body when DAB runtime config doesn't define a default value for that optional parameter.

    What do you think?

  17. Sean Leonard (seantleonard)
    assigned
    Sean Leonard (seantleonard)
    and unassigned
    rohkhann
    on Jul 25, 2024
  18. Sean Leonard (seantleonard)
    modified the milestones: Backlog, 1.3 on Jul 25, 2024
  19. Sean Leonard (seantleonard)
    moved this from Todo to In Progress in Data API builderon Jul 25, 2024
  20. Sean Leonard (seantleonard)
    added
    stored-procedure
    on Jul 25, 2024

  21. simonsabin commented on Jul 25, 2024

    @simonsabin
    Simon Sabin (simonsabin)
    on Jul 25, 2024 via email
    Collaborator

  22. seantleonard commented on Jul 25, 2024

    @seantleonard
    Sean Leonard (seantleonard)
    on Jul 25, 2024
    Contributor

    Hi Simon,

    I have a working branch here: https://github.com/Azure/data-api-builder/tree/dev/sean/sp_opt_1748

    I'd be happy to help you work on your suggestion:

    I think there is a case that needs to be dealt with and that’s a parameter that’s defined in the config that’s not supplied and no default.

    To clarify, when you say parameter defined in the config file, do you mean this? https://learn.microsoft.com/azure/data-api-builder/reference-configuration#parameters

    {
  "entities": {
    "<entity-name>": {
      ...
      "type": "stored-procedure",
      "parameters": {
        "<parameter-name-1>" : "<default-value>",
        "<parameter-name-2>" : "<default-value>",
        "<parameter-name-3>" : "<default-value>"
      }
    }
  }
}

    a value is required when defining params in the config. Perhaps you encountered a bug where an invalid, empty, or null value is provided for the parameter and DAB isn't handling properly?

  23. simonsabin commented on Jul 25, 2024

    @simonsabin
    Simon Sabin (simonsabin)
    on Jul 25, 2024
    Collaborator

    It’s just about what gets validated where and what’s controllable and how.
    Thinking about it, what is validating data types, default values.
    I as was pointed out there were a number of places in the code base this touched and on my investigation some didn’t have test cases thus the desire to understand how we put tests in place for this stuffs

  24. seantleonard commented on Jul 25, 2024

    @seantleonard
    Sean Leonard (seantleonard)
    on Jul 25, 2024
    Contributor

    I see. Do you have a few test cases in mind? That way I can better guide you were those can be added.

  25. Sean Leonard (seantleonard)
    linked a pull request that will close this issueDefer to database validation when default values for mandatory stored procedure params are excluded from config #2346on Aug 22, 2024
  26. Sean Leonard (seantleonard)
    closed this as completedin #2346on Sep 27, 2024
  27. Sean Leonard (seantleonard)
    added a commit that references this issue on Sep 27, 2024

    Defer to database validation when default values for mandatory stored…

    5b649ea
  28. github-project-automation
    moved this from Review In Progress to Done in Data API builderon Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingconfigchanges related to configgraphqlreststored-procedure

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions