Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node.js - Error Sanitisation & OData V4 Server Messages for Confirmation Popups in Fiori Elements #436

Closed
sebastianesch opened this issue Sep 22, 2023 · 13 comments
Assignees

Comments

@sebastianesch
Copy link
Collaborator

Hi,

I'm trying to get OData V4 Server messages to work https://ui5.sap.com/#/topic/fbe1cb5613cf4a40a841750bf813238e to get Confirmation Dialogs in my Fiori Elements applications (see https://ui5.sap.com/#/topic/9a536627a6a94de084b0605eb164d2c8), but it seems as CAP does not allow all properties SAPUI5 supports, but only the ones from the OData standard. Unfortunately SAPUI5 cannot work with "@transition" but expects "transition", which is sanitised by CAP.

It would make sense to state here

Additionally, the OData protocol specifies which properties an error object may have. If a custom property shall reach the client, it must be prefixed with `@` in order to not be purged.
that Confirmation Popups in Fiori Elements are not supported by CAP or the additional properties that SAPUI5 supports for errors should not be sanitised.

Kind regards,
Sebastian

@johannes-vogel
Copy link
Contributor

Hi @sebastianesch,

I feel that the sanitization is a bit too strict here.
As we do not support persistent error messages ootb, I assume we did not have the additional properties in mind.

We'll double check :)

Best regards,
Johannes

@johannes-vogel
Copy link
Contributor

HI @sebastianesch,

after additional reading of https://ui5.sap.com/#/topic/fbe1cb5613cf4a40a841750bf813238e section Messages in error responses

Only transition messages are transported in the error response. The messages may be bound or unbound. Error messages are always reported in the error response in JSON format, as described in the OData JSON Format Version 4.0 in Section 19 Error Response, with the following additions: ....

If I read that correctly, the transition property cannot be part of the error response. It can only be part of an error transported in sap-messages header or in the explicitly modeled message property of an entity.

Best,
Johannes

@sebastianesch
Copy link
Collaborator Author

Hi @johannes-vogel,

the section of the UI5 documentations above your quote states:

There are three different channels for transporting messages to the client:

  1. The error response for transporting unbound and bound transition messages in the error case.
  2. The sap-messages header for transporting unbound and bound transition messages in the success case.
  3. The message property as part of the entity for transporting bound state and transition messages in the success case.

For the confirmation use case, we report an HTTP Status Code 412, means we are in case 1 and I need the transition property in the response itself. The sap-messages header is used in the success case.

Looking at @sap/cds/libx/_runtime/common/error/constants.js:

  ALLOWED_PROPERTIES_MAP: { code: 1, message: 1, target: 1, details: 1, innererror: 1 },
  ADDITIONAL_MSG_PROPERTIES_MAP: { numericSeverity: 1, longtextUrl: 1, transition: 1 },

all the UI5 properties are there, but there is a distinction between allowed properties and additional properties - which would be useful to propagate to SAPUI5 applications.

After the error is handled in the in the Common Error module, it seems to be processed later again, but I have not figured out when and by which handler.

Kind regards,
Sebastian

@David-Kunz
Copy link
Contributor

Notes:

In the OData v4 error response documentation, there's no mention of the transition property. It only allows additional properties via annotations.

I can check with the Fiori Elements/UI5 colleagues.

@ThomasChadzelek
Copy link

Hello @sebastianesch !

I cannot comment on CAP, but I am the team architect for UI5's v4.ODataModel. From my perspective, the transition property is not needed for error responses because "Only transition messages are transported in the error response" - which means, we treat all of them as transition messages ;-)

Which issues do you experience with the confirmation use case? Maybe "Strict Handling" on https://openui5.hana.ondemand.com/topic/b54f7895b7594c61a83fa7257fa9d13f helps?

Best regards,
Thomas

@sebastianesch
Copy link
Collaborator Author

sebastianesch commented Feb 15, 2024

HI @David-Kunz @ThomasChadzelek,

I have created an example at https://github.com/sebastianesch/sap-cap-examples/tree/master/412-confirmation-dialog. At the moment, if I reply in CAP with an 412 error, the Fiori Elements application treats the response as an error and does not show a confirmation dialog. (See repo for a screenshot)

Looking at the UI5 documentation Confirmation Popup for Actions that Fail with 412 Warnings, it states:

Depending on the settings of the back end, there are two options:

  • If the preference is unknown, it's ignored and the action is executed as usual.
  • If the preference is known, application developers can configure the back end to send a 412 message ("Precondition Failed" message).

At the bottom of the page (for OData V4) is stated:

You must configure 412 messages from the back end as transition messages, not as state messages.

That's why I figured, that the response needs the transition property.

@David-Kunz I'm aware that the OData V4 spec does not describe the transition property, but the Server Messages in the OData V4 Model documentation does.

Kind regards,
Sebastian

@ThomasChadzelek
Copy link

"if I reply in CAP with an 412 error, the Fiori Elements application treats the response as an error"
Are you sure that your response includes "Preference-Applied : handling=strict"? W/o that, the client would not know that the error has been caused by strict handling.

@sebastianesch
Copy link
Collaborator Author

sebastianesch commented Feb 16, 2024

@ThomasChadzelek I don't do that. I haven't seen this in the documentation anywhere. I'll try that.

Adding the header to the response results in the Fiori App staying in the busy state:

image

@ThomasChadzelek
Copy link

Hmm, the response now looks as expected, as far as I can tell. Maybe you can share the full response as text here.

Are there any errors on console, apart from that "busy lock"?

@sebastianesch
Copy link
Collaborator Author

The response from the network tab in Chrome is:

--batch_id-1708089594292-130
content-type: application/http
content-transfer-encoding: binary

HTTP/1.1 412 Precondition Failed
odata-version: 4.0
preference-applied: handling=strict
content-type: application/json;odata.metadata=minimal;IEEE754Compatible=true

{"error":{"code":"412","message":"Precondition Failed: Confirm project closure","@Common.numericSeverity":4}}
--batch_id-1708089594292-130--

When I delete the console before executing the action, I don't get any output in the Chrome Console immediately. After 30 seconds I get:

Log-dbg.js:499 2024-02-16 14:21:35.810500 busy lock for id-1708089648579-17/busy with value 1 timed out after 30 seconds! -  
g @ Log-dbg.js:499
t.error @ Log-dbg.js:249
(anonymous) @ BusyLocker.ts:57
setTimeout (async)
s @ BusyLocker.ts:56
_updateLock @ BusyLocker.ts:94
lock @ BusyLocker.ts:80
e @ TransactionHelper.ts:74
onSubmitted @ TransactionHelper.ts:912
S @ facade.ts:1850
(anonymous) @ facade.ts:1894
X @ facade.ts:1721
(anonymous) @ facade.ts:406
H @ facade.ts:220
_ @ facade.ts:110
e @ TransactionHelper.ts:902
t @ EditFlow.ts:1979
await in t (async)
(anonymous) @ ControllerExtension-dbg.js:104
t @ TableAPI.ts:1489
n.<computed> @ ClassSupport.ts:300
y @ ExpressionParser-dbg.js:412
(anonymous) @ ExpressionParser-dbg.js:735
l @ ExpressionParser-dbg.js:915
l.getExternalValue @ CompositeBinding-dbg.js:309
u @ EventHandlerResolver-dbg.js:349
(anonymous) @ EventHandlerResolver-dbg.js:173
i.fireEvent @ EventProvider-dbg.js:241
c.fireEvent @ Element-dbg.js:659
(anonymous) @ ManagedObjectMetadata-dbg.js:826
S.ontap @ Button-dbg.js:599
c._handleEvent @ Element-dbg.js:345
N._handleEvent @ UIArea-dbg.js:1050
dispatch @ jquery-dbg.js:5430
c @ jquery-mobile-custom-dbg.js:1907
d @ jquery-mobile-custom-dbg.js:2030
dispatch @ jquery-dbg.js:5430
y.handle @ jquery-dbg.js:5234
trigger @ jquery-dbg.js:8823
(anonymous) @ jquery-dbg.js:8901
each @ jquery-dbg.js:385
each @ jquery-dbg.js:207
trigger @ jquery-dbg.js:8900
j @ jquery-mobile-custom-dbg.js:1455
L @ jquery-mobile-custom-dbg.js:1465
dispatch @ jquery-dbg.js:5430
y.handle @ jquery-dbg.js:5234

@ThomasChadzelek
Copy link

OK, our docs are for sure never precise enough ;-) The response is insufficient. I cannot exactly explain how it leads to the issue you observe, but I can explain what is missing.

When an action fails due to "strict handling", it means that the action would have succeeded without strict handling, but that warnings occured which have been escalated, so to say, to become errors instead (that's the core of strict handling). This means that the error response needs to contain an error object (check) with all these warnings as details - this is missing. Here is an example of such details from our integration test (lines 47920ff.):

  		details : [{
  			"@Common.numericSeverity" : 3,
  			code : "CODE1",
  			"@SAP__core.ContentID" : "0.0",
  			message : "Note is empty",
  			target : "SalesOrder/Note"
  		}, {
  			"@Common.numericSeverity" : 3,
  			code : "CODE1",
  			"@SAP__core.ContentID" : "0.0",
  			message : "Note is empty",
  			target : "SalesOrder/Note"
  		}, {
  			"@Common.numericSeverity" : 2,
  			code : "CODE2",
  			"@SAP__core.ContentID" : "0.0",
  			message : "Some unbound info"
  		}]

@333sachin
Copy link

not sure, if this is related, but I get this error when adding composition child entity to a main entity
image

@renejeglinsky
Copy link
Contributor

Hi @333sachin ,
is needed, please open a new issue. Most probably for your issue a question in the community would be best.
As the initial topic is solved in the meantime, I close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants