Fixed to use apierror.APIError instead of spanner.Error

[yukia3e]

Dear Developers.

I noticed that Yo is using DEPRECATED spanner.Error.
So I have modified it to use APIError.APIError, as exemplified in the spanner package.
ref. https://pkg.go.dev/cloud.google.com/go/spanner#Error

I also re-created various templates with the modified contents in the Docker image environment specified in .circleci/config.yml.

This is my first contribution to Yo. So I apologize if I skipped any necessary steps.
Thank you in advance for your review.

[kazegusuri]

Sorry for the late reply and thank you for the contribution. I didn't notice the spanner.Error's code will be deprecated.

The spanner library depends on spanner.Error type before so I wrapped an error inside yo by spanner.Error. The spanner.Error's Code is deprecated in favor of spanner.ErrCode(). It means the code is retrieved from error by status.FromError() as the common way for gRPC.
However some functions in the library such as ErrDesc() or extractResourceType() depend on spanner.Error type. I'm not confident we can use other error type than spanner.Error. I think it's okay but please let me think for a while.

btw if we can use other error type, I think we don't need to apierror.APIError. We can just implement an interface for status.FromError() for an error. Is there a reason or a library that uses apierror.APIError instead of spanner.Error?

[kazegusuri]

Is there a reason or a library that uses apierror.APIError instead of spanner.Error?

Ah ignore me. I read the comment in spanner.Error.

[yukia3e]

Thanks for getting back to me!

I noticed the deprecation of spanner.Error when a file output by Yo failed the latest staticcheck check. So, not being familiar with the background of this, I looked into the discussion and found that it was fixed in the following pull request.

refactor(spanner): return APIError as wrapped error in spanner.Error struct

In the review comments in the above PR, I found the following statement.

This example code will work both when the client returns instances of spanner.Error and when it starts to return apierror.APIError instances.

It may be difficult to determine from this information alone, but I hope it is helpful.
Thank you for your continued review.

[kazegusuri]

apierror.APIError implements GRPCStatus() so it is enough to just call status.Convert(ae) without unwrap. (not sure why we use unwrap for spanner.Error)

[yukia3e]

Thank you for pointing this out. I have corrected the issue in 464537e .

[kazegusuri]

sorry for so late again. I checked the implementations.

I have a concern in the proposed implementation. if a user of Yo uses old spanner library that hasn't introduced apierrors.Error, it makes an unexpected behavior. apierrors.Error was introduced since cloud.google.com/go/spanner v1.28.0. It is about 4 months ago. So the issue may not happen.

I leave a comment but LGTM.

[yukia3e]

Thanks for your review and comments! I have removed the unnecessary Unwrap implementation that you pointed out in your review.

I apologize for the inconvenience and I would be happy to review it again. Thank you so much.

[kazegusuri]

Thank you for your contribution!

[yukia3e]

@kazegusuri
I believe the fixes merged in this PR are not yet included in the release, is there anything additional I should do to have them included in the release?
(I am not used to contributing to OSS and apologize if I have omitted any work...)

If a release is planned, I would appreciate it if you could let me know when it will be available.

Thank you for your cooperation!

[kazegusuri]

@yukia3e sorry. 🙏 I just released a new version.
https://github.com/cloudspannerecosystem/yo/releases/tag/v0.5.3

[yukia3e]

Thank you, I'm glad I could contribute to yo!