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

GetEnterpriseUsersWithMarkerAsync throws NRE when evaluating Entries #948

Open
4 tasks done
watfordsuzy opened this issue May 17, 2024 · 13 comments
Open
4 tasks done

Comments

@watfordsuzy
Copy link
Contributor

Description of the Issue

If I include an externalAppUserId to a call to GetEnterpriseUsersWithMarkerAsync the resulting marker based collection has a null Entries field.

Steps to Reproduce

string? marker = null;
do
{
    var result = await client.UsersManager.GetEnterpriseUsersWithMarkerAsync(marker, externalAppUserId: "user_id");
    foreach (BoxUser user in result.Entries) // NullReferenceException (result.Entries)
    {
        // ...
    }

    marker = result.NextMarker;
}
while (!string.IsNullOrWhiteSpace(marker));

Expected Behavior

The list of entries is enumerated.

Error Message, Including Stack Trace

This is in a durable function app and the stack trace is a bit useless.

Screenshots

Versions Used

.Net SDK: 5.7.0
Windows: dotnet-isolated|8.0

@watfordsuzy
Copy link
Contributor Author

Worse still, calling GetEnterpriseUsersAsync also throws an error when I include the external app ID.

@watfordsuzy
Copy link
Contributor Author

watfordsuzy commented May 17, 2024

I now think this is a regression with the main Box API, how do I get this escalated?

https://gist.github.com/watfordsuzy/c5f85f1099e259f64e5acb4b0881d18f

ref: https://developer.box.com/guides/api-calls/pagination/marker-based/

@mwwoda
Copy link
Contributor

mwwoda commented May 17, 2024

I just checked and this endpoint behaves very strangely with different payload. When useMarker is included along with external_app_user_id, instead of returning collection in entries field as usual it returns it in the users field, which is not even present in the spec. As a result SDK cannot deserialize it because of field name mismatch and entries field is null.

I could only reproduce this issue only when used marker so far (couldn't reproduce with GetEnterpriseUsersAsync only with GetEnterpriseUsersMarkerBasedAsync). At the first glance, response in your example from GetEnterpriseUsersAsync looks fine. Not sure why you got NRE here.

You could try contacting Box support at https://forum.box.com/top?period=quarterly. I'll contact the service owners on our side to get first-hand feedback.

@mwwoda
Copy link
Contributor

mwwoda commented May 17, 2024

You also mentioned regression? Could you elaborate on that? Was it working fine for you in the past?

@watfordsuzy
Copy link
Contributor Author

watfordsuzy commented May 17, 2024

Yes, all of this code was working for me previously. Seems only recently that the addition of external_app_user_id causes the weird response from the server.

EDIT: I've also raised an issue on the new support forums. https://support.box.com/hc/en-us/community/posts/29468435097747-Users-API-regression-with-usemarker-parameter

@watfordsuzy
Copy link
Contributor Author

It looks like you were correct and I can successfully work around the issue using GetEnterpriseUsersAsync. Not 100% sure why I saw the NRE, might have been an old instance of the code was picked up by the function app host.

@mwwoda
Copy link
Contributor

mwwoda commented May 20, 2024

Service owners confirmed the issue and they looking into a fix. I don't have any timeframe for now, but I'll let you know in case of any updates. For now, my advice is to stick with GetEnterpriseUsersAsync function.

@watfordsuzy
Copy link
Contributor Author

@mwwoda thank you, will do!

Is it even possible to have two box users with the same external_app_user_id?

@congminh1254
Copy link
Member

Hi @watfordsuzy

As I tested, so it's not possible to have two Box users with the same external_app_user_id.

@watfordsuzy
Copy link
Contributor Author

@mwwoda @congminh1254 I received the following from Box Support on my case and I'm not sure how to manage this with the C# library:

Case: #3099758
Hi Christopher,

Thanks for reaching out. My name is Dan and I'm a Platform specialist here at Box. I understand then when using the .NET SDK, that when you pass a external_app_user_id parameter along with a usermarker=true parameter to a List Enterprise Users call that marker-based pagination is not being used.

When using marker-based pagination, a marker field will only be returned in the response when there are enough entries to warrant multiple pages of results. Where an external_app_user_id is specified only a single entry will ever be returned, and so a marker field will not be included in the results even when marker-based pagination is specified.

I hope this is helpful. Please let me know if you have any questions or if I may be of any further assistance.

I asked a follow-up question about the stability of the APIs when requesting Marker Based results and received the following:

Hi Christopher,

Inasmuch as it's not always possible to know whether enough entries will be returned to allow for multiple page of results, it will not always be definitively possible to know whether results will be returned in a marker-based format. In this connection, the best way forward will be to write the program in such a way as to be able to useful parse results in either format.

I can definitely see how it would be useful for the API to always return results in the same format when marker based pagination is specified, even in the event that only a single page of results is returned. You can flag the need for this change at pulse.box.com. Our engineers regularly review this platform to inform their future priorities and roadmaps.

Copy link

stale bot commented Jun 28, 2024

This issue has been automatically marked as stale because it has not been updated in the last 30 days. It will be closed if no further activity occurs within the next 7 days. Feel free to reach out or mention Box SDK team member for further help and resources if they are needed.

@stale stale bot added the stale label Jun 28, 2024
@watfordsuzy
Copy link
Contributor Author

@mwwoda @congminh1254 thoughts on this? I think the SDK should be modified such that this doesn't break unexpectedly. Either by:

  1. Not allowing the external app ID when you use marker based pagination
  2. "Fixing" the response from the API to match the contract of the marker parameter (very bizarre to me the API gets to ignore that field in certain situations).

@stale stale bot removed the stale label Jul 1, 2024
@mwwoda
Copy link
Contributor

mwwoda commented Jul 3, 2024

@watfordsuzy
I just pinged the team to check the state of the fix on the backend. Ideally, using externalAppId should not lead to this behaviour, and SDK fix should not be necessary. If it cannot be fixed on the service side for some reason, then we need to adjust the SDK.

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

No branches or pull requests

8 participants