Canadapost cancel error (Void) is not behing catched

[jacobshilitz]

Describe the bug
Canada post does not catch a void cancel error with empty message

<?xml version="1.0" encoding="UTF-8"?>

<messages xmlns="http://www.canadapost.ca/ws/messages"/>

as the docs say it return 404. I can't see the status code to be passed to the parse_error_response method

in addtion, we need to check if a menifest was already ishued, and if yes we need to do a shipmentrefund becouse a [voidshipment](https://www.canadapost-postescanada.ca/info/mc/business/productsservices/developers/services/shippingmanifest/voidshipment.jsf
l) is not allowed after a manifest was issued

Wouldn't checking status codes be a good practice?

[danh91]

I believe at the API level, once you have a manifest attached to a shipment it doesn't allow cancelling the shipment 🤔.

This one might require some API logs or a way to reproduce to debug it because the current implementation is supposed to cover the case described above:

karrio/modules/connectors/canadapost/karrio/mappers/canadapost/proxy.py

Lines 106 to 192 in 5b235d9

	def cancel_shipment(self, requests: lib.Serializable) -> lib.Deserializable:
	# retrieve shipment infos to check if refund is necessary
	infos: typing.List[typing.Tuple[str, str]] = lib.run_asynchronously(
	lambda shipment_id: (
	shipment_id,
	lib.request(
	url=f"{self.settings.server_url}/rs/{self.settings.customer_number}/{self.settings.customer_number}/shipment/{shipment_id}",
	trace=self.trace_as("xml"),
	method="GET",
	headers={
	"Content-Type": "application/vnd.cpc.shipment-v8+xml",
	"Accept": "application/vnd.cpc.shipment-v8+xml",
	"Authorization": f"Basic {self.settings.authorization}",
	"Accept-language": f"{self.settings.language}-CA",
	},
	),
	),
	requests.serialize(),
	)
	# make refund requests for submitted shipments
	refunds: typing.List[typing.Tuple[str, str]] = lib.run_asynchronously(
	lambda payload: (
	payload["id"],
	(
	lib.request(
	url=f"{self.settings.server_url}/rs/{self.settings.customer_number}/{self.settings.customer_number}/shipment/{payload['id']}/refund",
	trace=self.trace_as("xml"),
	data=payload["data"],
	method="POST",
	headers={
	"Content-Type": "application/vnd.cpc.shipment-v8+xml",
	"Accept": "application/vnd.cpc.shipment-v8+xml",
	"Authorization": f"Basic {self.settings.authorization}",
	"Accept-language": f"{self.settings.language}-CA",
	},
	)
	if payload["data"] is not None
	else payload["info"]
	),
	),
	[
	dict(
	id=_,
	info=info,
	data=provider_utils.parse_submitted_shipment(info, requests.ctx),
	)
	for _, info in infos
	],
	)
	# make cancel requests for non-submitted shipments
	responses: typing.List[typing.Tuple[str, str]] = lib.run_asynchronously(
	lambda payload: (
	payload["id"],
	(
	lib.request(
	url=f"{self.settings.server_url}/rs/{self.settings.customer_number}/{self.settings.customer_number}/shipment/{payload['id']}",
	trace=self.trace_as("xml"),
	method="DELETE",
	headers={
	"Content-Type": "application/vnd.cpc.shipment-v8+xml",
	"Accept": "application/vnd.cpc.shipment-v8+xml",
	"Authorization": f"Basic {self.settings.authorization}",
	"Accept-language": f"{self.settings.language}-CA",
	},
	)
	if payload["refunded"]
	else payload["response"]
	),
	),
	[
	dict(
	id=_,
	response=response,
	refunded=(
	getattr(lib.to_element(response), "tag", None)
	!= "shipment-refund-request-info"
	),
	)
	for _, response in refunds
	],
	)
	return lib.Deserializable(
	responses, lambda ___: [(_, lib.to_element(__)) for _, __ in ___]
	)

[jacobshilitz]

You are right, i missed this one

i rechecked the code, and it seems that the problem is that the api should send shipment-id and not the tracking code

<shipment-id>25208172608**9372</shipment-id>
<shipment-status>created</shipment-status>
<tracking-pin>80464771409**36</tracking-pin>

i think the bug is here, and that shipment_identifier should save shipment_id not the tracking pin

karrio/modules/connectors/canadapost/karrio/providers/canadapost/shipment/create.py

Lines 50 to 51 in 692efcc

	tracking_number=info.tracking_pin,
	shipment_identifier=info.tracking_pin,

see PR #683

in any case the 404 with empty message was not catched as a error (there was 2 404 in a row)

[jacobshilitz]

the second bug which I don't know how to fix or if it needs a fix, that when you cancel on the dashboard, how do you pass a email?

also it should throw a error if no email, because it's a required field

maybe we should have a special setting in the carrier level.

karrio/modules/connectors/canadapost/karrio/providers/canadapost/shipment/cancel.py

Line 52 in 692efcc

dict(email=payload.options.get("email")),

[jacobshilitz]

the third bug is that after hardcoding my email it did open a ticket, but the cancel step still tried to run.

i'm emailing you the logs

[jacobshilitz]

I think i found one of the bugs when you try to cancel and refund request was done the parsing is buggy with 2 bugs.

first here is a successful refund reply

<?xml version="1.0" encoding="UTF-8"?>
  <shipment-refund-request-info xmlns="http://www.canadapost.ca/ws/shipment-v8">
  <service-ticket-date>2024-09-11</service-ticket-date>
  <service-ticket-id>015***7353</service-ticket-id>
</shipment-refund-request-info>

but this code that is checking if it was refunded is checking a condition that never happen

karrio/modules/connectors/canadapost/karrio/mappers/canadapost/proxy.py

Lines 181 to 184 in 36217a0

	refunded=(
	getattr(lib.to_element(response), "tag", None)
	!= "shipment-refund-request-info"
	),

something is wrong with this logic because when you run this python code manually it return a string {http://www.canadapost.ca/ws/shipment-v8}shipment-refund-request-info not shipment-refund-request-info (let me know how we fix this, or if we test against the full url)

getattr(lib.to_element('<?xml version="1.0" encoding="UTF-8"?><shipment-refund-request-info xmlns="http://www.canadapost.ca/ws/shipment-v8"><service-ticket-date>2024-09-11</service-ticket-date><service-ticket-id>0157**53</service-ticket-id></shipment-refund-request-info>'), "tag", None)

bug 2 in there is a case where the refund was already done, and at that case we also don't need to void, but rather return this error to the user, not a false error that the shipment is not in good status

The response when you refund a order that already had a refund request

<?xml version="1.0" encoding="UTF-8"?>
  <messages xmlns="http://www.canadapost.ca/ws/messages">
    <message>
      <code>7292</code>
      <description>A refund has already been requested for this shipment.  Note that refund requests may take a few days to be processed.</description>
    </message>
  </messages>

[jacobshilitz]

I've been playing around with the code, and I think I've fixed the problems so far. However, I'm afraid you won't like my solution, and I'm sure you'll have a better way to achieve the same result.

                    refunded=(
                        "shipment-refund-request-info" not in getattr(lib.to_element(response), "tag", None)
                        and "messages" not in getattr(lib.to_element(response), "tag", None)
                    ),

but it's not so good because code 7291 means you should use void, so we should just check if its 7292 and 7296